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

Changing round to ceil in calculate_seconds method and including minimum remain rate limit configurable #184

Open
wants to merge 41 commits into
base: master
Choose a base branch
from

Conversation

joaopamaral
Copy link
Contributor

Description of change

The round call in calculate_seconds method was making the retry attempt trigger before resetting the API token for less than a second (when the API rate limit is achieved), which causes a new exception that causes the extract to fail. The ceil fix this issue since it makes the request always being made after the reset.

The minimum remaining rate limit parameter was included to add the possibility of setting a custom limit remaining rate limit instead of 0. With that config, it's possible to avoid reaching the extreme limit (0 remaining rate limit) and it's important if there are others tokens used by the same user.

Manual QA steps

Risks

Rollback steps

  • revert this branch

@singer-bot
Copy link
Collaborator

Hi @joaopamaral, thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes.

@singer-bot
Copy link
Collaborator

You did it @joaopamaral!

Thank you for signing the Singer Contribution License Agreement.

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.

2 participants