Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTTP Requests response status code handlings #42

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions src/api/rate_limiter.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,22 @@ def __init__(self, request, log=None, message="", msg_args=()):
message = f"Request %s: %s + %s {message}"
log(message, request.method, request.url, request.cookies)

class RLRequestFailed(RateLimiterError):
def __init__(self, request, *args, **kwargs):
super().__init__(request, *args, **kwargs)
Comment on lines +25 to +27
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I'm not mistaken the whole point of this exception is to allow future subclassing (because today only RLRequestClientError inherits from it)
However it made the code a bit more obscure (especially because you except the parent class not this one)
So in the interest of readability maybe we could get rid of this one and instead except RLClientError later?


class RLRequestClientError(RLRequestFailed):
def __init__(self, request, *args, **kwargs):
super().__init__(request, *args, **kwargs)
Comment on lines +25 to +31
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe RLClientError, RLClientError4xx would be a better name?
(same with Server)

Comment on lines +29 to +31
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe RLClientError, RLClientError4xx would be a better name?
(same with Server)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also the name RLRequestServerError wrongly give the impression of inheriting from RLRequestFailed


class RLErrorWithPause(RateLimiterError):
def __init__(self, request, time_to_wait, *args, **kwargs):
super().__init__(request, *args, **kwargs)
self.time_to_wait = time_to_wait

class RLRequestServerError(RLErrorWithPause):
def __init__(self, request, time_to_wait, *args, **kwargs):
super().__init__(request, time_to_wait, *args, **kwargs)

# pylint: disable=too-few-public-methods
class RequestEntry:
Expand Down Expand Up @@ -101,6 +111,21 @@ def handle_get_request(self, request):
match resp.status_code:
case 200:
return resp.json()
case 400, 401, 403, 404, 405:
# HTTP failure client side
raise RLRequestClientError(
request,
self.logger.error,
f"40X error code, client side error, useless to retry."
)
case 500, 501, 502, 503, 504:
# HTTP failure server side
raise RLRequestServerError(
request,
300,
self.logger.error,
f"50X error code, server side error, retry in 5 min ..."
)
case 429:
try:
time_to_wait = int(resp.headers["Retry-After"])
Expand Down Expand Up @@ -163,6 +188,14 @@ async def handle_requests(self):

try:
self.requests[request.key]["result"] = self.handle_get_request(request)
except RLRequestFailed as exc:
# Request failed and should not be sent again
self.requests[request.key]["result"] = None
# set the exception
self.requests[request.key]["exception"] = (
RLRequestFailed(request, self.logger.error, "Request Failed"),
exc,
)
except RateLimiterError as exc:
if isinstance(exc, RLErrorWithPause):
await self.wait(exc.time_to_wait)
Expand Down
30 changes: 25 additions & 5 deletions src/api/rootme_api.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import logging

from os import getenv
from api.rate_limiter import RateLimiter
from api.rate_limiter import RLRequestFailed, RateLimiter

class RootMeAPIError(Exception):
def __init__(self, *args: object) -> None:
super().__init__(*args)

class RootMeAPIManager:
"""
Expand All @@ -18,6 +23,7 @@ def __init__(self, rate_limiter: RateLimiter):
else:
raise RuntimeError("API_URL is not set.")
self.rate_limiter = rate_limiter
self.logger = logging.getLogger(__name__)

def get_rate_limiter(self):
return self.rate_limiter
Expand All @@ -28,11 +34,17 @@ async def get_challenge_by_id(self, _id):
-> returns the raw json for now
"""
# use the api_key in the cookies
request_url = f"{self.API_URL}/challenges/{_id}"
cookies = {"api_key": self.API_KEY.strip('"')}
request_method = "GET"
# ask the rate limiter for the request
data = await self.rate_limiter.make_request(
f"{self.API_URL}/challenges/{_id}", cookies, "GET"
)
try:
data = await self.rate_limiter.make_request(
request_url, cookies, request_method
)
except RLRequestFailed as exc:
self.logger.error("%s Request to get challenge %s failed.", request_method, request_url)
raise RootMeAPIError() # Handle the fact the request is wrong
return data

async def get_user_by_id(self, _id):
Expand All @@ -41,7 +53,15 @@ async def get_user_by_id(self, _id):
-> returns the raw json for now
"""
# use the api_key in the cookies
request_url = f"{self.API_URL}/auteurs/{_id}"
cookies = {"api_key": self.API_KEY.strip('"')}
request_method = "GET"
# ask the rate limiter for the request
data = await self.rate_limiter.make_request(f"{self.API_URL}/auteurs/{_id}", cookies, "GET")
try:
data = await self.rate_limiter.make_request(
request_url, cookies, request_method
)
except RLRequestFailed as exc:
self.logger.error("%s Request to get user %s failed.", request_method, request_url)
raise RootMeAPIError()
return data
22 changes: 18 additions & 4 deletions src/database/db_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import sqlite3
from os import getenv, path

from api.rootme_api import RootMeAPIError
from database.db_structure import (
sql_create_user_table,
sql_add_user,
Expand Down Expand Up @@ -64,8 +65,11 @@ async def add_user(self, idx):
if self.has_user(idx):
return None

# Retreive information from RootMe API
raw_user_data = await self.api_manager.get_user_by_id(idx)
# Retrieve information from RootMe API
try:
raw_user_data = await self.api_manager.get_user_by_id(idx)
except RootMeAPIError:
return None
user = User(raw_user_data)

cur.execute(sql_add_user, user.to_tuple())
Expand Down Expand Up @@ -102,15 +106,25 @@ async def fetch_user_new_solves(self, idx):
if user is None:
raise InvalidUser(idx, "DatabaseManager.fetch_user_new_solves: User %s not in database")

raw_user_data = await self.api_manager.get_user_by_id(idx)
try:
raw_user_data = await self.api_manager.get_user_by_id(idx)
except RootMeAPIError:
# If for some reason we can get the user on this iteration
# we will get him next time maybe ...
self.logger.error("User %s could not be fetch from the API, yet we keep running", idx)
return
user.update_new_solves(raw_user_data)
if not user.has_new_solves():
self.logger.debug("'%s' hasn't any new solves", user)
return

self.logger.info("'%s' has %s new solves", user, user.nb_new_solves)
for challenge_id in user.yield_new_solves(raw_user_data):
challenge_data = await self.api_manager.get_challenge_by_id(challenge_id)
try:
challenge_data = await self.api_manager.get_challenge_by_id(challenge_id)
except RootMeAPIError:
# If we can't fetch the challenge, sadly there is not much we can do
continue
challenge = Challenge(challenge_id, challenge_data)
self.logger.debug("'%s' solved '%s'", repr(user), repr(challenge))
yield challenge
Loading