-
-
Notifications
You must be signed in to change notification settings - Fork 135
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
Adding retry logic and connection pooling may improve stability #41
Comments
I have considered this but I want to maintain a line of seperation between what the user implements and what is handled by the package. I.e. in this case a user may want to do something else if the call fails, e.g. if their internet is down then this call could repeat infinitely or not retry enough times. Also, you can add the parameter And yes, I do plan on using session requests instead of the current method, however I am currently very busy this week. I intend on releasing an update that implements this before the end of this month. |
After doing quite a bit of research it appears that sessions are not thread safe. Since a single client could be shared across threads it might not be a good idea to use one session across requests. I also don't know how stable they are, I considered making a new session every time a new access token is granted. |
Yeah, that is something important to consider. If you still wanted allow users to leverage connection pooling you could probably take a requests.session as an optional constructor argument and use the session if it exists. You might be able also use httpx to create a thread safe session, but that is probably a larger refactor from what you have now. |
Hi Tauheed and Tyler, I enjoyed reading this discussion—both of you bring great points! Tauheed, the idea of passing a requests.session as an optional argument is a smart way to add flexibility, and Tyler, I appreciate the focus on keeping user logic separate from the package. I’d love to collaborate and help contribute to the project. If either of you is open to a quick call to brainstorm or discuss next steps, feel free to reach out to me! |
Hey! I've been using this package for the past month or so and it's helped me hit the ground running.
I've noticed at times there are http timeout errors (likely my logic) and part of my strategy right now is to wait and retry a set number of times with exponential backoff.
To that end, it may be useful if the client had retry logic for timeout errors built in so that users don't need to manage that themselves. At least for GET requests.
Also, how can I contribute to this project?
The text was updated successfully, but these errors were encountered: