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

Amount of retries configurable #10

Conversation

shaharyar-shamshi
Copy link
Contributor

@shaharyar-shamshi shaharyar-shamshi commented Nov 17, 2022

Fixes #7

@CLAassistant
Copy link

CLAassistant commented Nov 17, 2022

CLA assistant check
All committers have signed the CLA.

@shaharyar-shamshi shaharyar-shamshi force-pushed the feat/error_429_retries branch 2 times, most recently from 57e4edf to b4bb87b Compare November 17, 2022 14:40
@shaharyar-shamshi shaharyar-shamshi marked this pull request as draft November 17, 2022 14:49
@shaharyar-shamshi shaharyar-shamshi marked this pull request as ready for review November 17, 2022 15:09
@shaharyar-shamshi
Copy link
Contributor Author

@lullis if you can kindly review it

@shaharyar-shamshi shaharyar-shamshi force-pushed the feat/error_429_retries branch 2 times, most recently from 2e5cf3b to 3d2f9ff Compare November 17, 2022 18:05
@shaharyar-shamshi shaharyar-shamshi changed the title amount of retries configurable Amount of retries configurable Nov 17, 2022
@@ -20,6 +21,12 @@ class ShipBobClient(Session):

ACCESS_TOKEN = os.getenv("SHIPBOB_ACCESS_TOKEN")

MAX_RETRIES_AFTER_RATE_LIMIT: int = os.getenv("SHIPBOB_MAX_RETRIES_AFTER_RATE_LIMIT")
Copy link

Choose a reason for hiding this comment

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

Instead of having a limit of amount of times to retry, wouldn't it be better/easier to track the amount of time since the last successful request?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lullis it is being handled this is maximum number of retries to perform and I am mainting the separate counter RETRIES_AFTER_RATE_LIMIT to count number of time since last successful request

@shaharyar-shamshi
Copy link
Contributor Author

@lullis if you can review this PR

@shaharyar-shamshi
Copy link
Contributor Author

@lullis do let me if I need to change anything more

Signed-off-by: Shaharyar Shamshi <shaharyarshamshi@gmail.com>
@shaharyar-shamshi
Copy link
Contributor Author

@lullis do I need to update anything

@shaharyar-shamshi shaharyar-shamshi requested a review from lullis March 8, 2023 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add proper rate-limit and error controls to Shipbob client class
4 participants