You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The main problem with this code is the management of self.__session and self.__headers. If two coroutines call the request method at the same time, both might attempt to modify these variables, leading to undefined behavior.
Consider not using '' for empty value checks, but None instead. In the __init__ method, you have a default timeout timeout: int = 5, but if the timeout is None or 0, you set it to 10.
I can't quite grasp the purpose of headers here. I believe passing headers directly to aiohttp.ClientSession during initialization might be a more concise and intuitive approach.
Regarding the timeout exception handling I previously raised, I'm quite puzzled when looking at the code below. I've also checked the official documentation of aiohttp but couldn't find any guidance on handling the timeout exceptions thrown by aiohttp (which is also a reason I really dislike aiohttp).
Flawed Loop Logic
https://github.com/mrwan200/EnkaNetwork.py/blob/728f5ca42299c315f52993323e823f97c27c2dbf/enkanetwork/http.py#L121
When I set
RETRY_MAX
to 2,tries
only gets assigned the values 0 or 1.As a result, the following code
tries > RETRY_MAX
will always befalse
. Meaning the code block after theif
statement will never be executed.https://github.com/mrwan200/EnkaNetwork.py/blob/728f5ca42299c315f52993323e823f97c27c2dbf/enkanetwork/http.py#L144-L145
Coroutine Unsafe
The main problem with this code is the management of
self.__session
andself.__headers
. If two coroutines call therequest
method at the same time, both might attempt to modify these variables, leading to undefined behavior.https://github.com/mrwan200/EnkaNetwork.py/blob/728f5ca42299c315f52993323e823f97c27c2dbf/enkanetwork/http.py#L108-L119
Default Parameters
Consider not using
''
for empty value checks, butNone
instead. In the__init__
method, you have a default timeouttimeout: int = 5
, but if the timeout isNone
or0
, you set it to10
.https://github.com/mrwan200/EnkaNetwork.py/blob/728f5ca42299c315f52993323e823f97c27c2dbf/enkanetwork/http.py#L83-L86
My recommended improvement:
Concerning
headers
I can't quite grasp the purpose of
headers
here. I believe passingheaders
directly toaiohttp.ClientSession
during initialization might be a more concise and intuitive approach.https://github.com/mrwan200/EnkaNetwork.py/blob/728f5ca42299c315f52993323e823f97c27c2dbf/enkanetwork/http.py#L108-L112
Session Management
__session
is assigned and cleared across different methods. I'd suggest considering the use of anAsyncContextManager
for this purpose.The text was updated successfully, but these errors were encountered: