Skip to content

Update max rate-limiting metadata #36734

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Apr 3, 2024
Merged

Update max rate-limiting metadata #36734

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
944a87a
update github
ChristoGrab Apr 1, 2024
5813e18
update sentry
ChristoGrab Apr 1, 2024
15a9d92
update zendesk support
ChristoGrab Apr 1, 2024
3e05f3a
add note on rate limit window
ChristoGrab Apr 3, 2024
4ae56b7
format
ChristoGrab Apr 3, 2024
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
2 changes: 1 addition & 1 deletion airbyte-integrations/connectors/source-github/metadata.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ data:
githubIssueLabel: source-github
icon: github.svg
license: MIT
maxSecondsBetweenMessages: 5400
maxSecondsBetweenMessages: 3600
name: GitHub
remoteRegistries:
pypi:
Expand Down
2 changes: 1 addition & 1 deletion airbyte-integrations/connectors/source-sentry/metadata.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ data:
githubIssueLabel: source-sentry
icon: sentry.svg
license: MIT
maxSecondsBetweenMessages: 64800
maxSecondsBetweenMessages: 60
Copy link
Contributor

Choose a reason for hiding this comment

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

Checking the Sentry API doc, it seems like the rate limit window is a second: We restrict both how frequently a request is made (requests per second rate limit) <...>. What is the reasoning for having 60 seconds here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was originally a bit confused by this as well, but when testing and viewing the response headers, the x-sentry-rate-limit-reset was being set to the next full minute from the request (so a request sent at 12:00:02 will return a reset time of 12:01:00, as will a request from 12:00:36). Worth noting that I ran some more tests and some of the streams do seem to have smaller windows, but the events stream in particular consistently sets this value to the next full minute.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we document this somewhere as it would be interesting to know where this comes from. I'm also curious is HTTP requests to sentry can take more than a minute, maxSecondsBetweenMessages should be increased. I don't know how big the responses can be with big datasets but there is that to consider

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure thing, just added a comment for clarity 👍 I agree this value may actually be higher given it's not as set in stone as their docs would suggest, but my understanding is also that at the moment we don't actually do anything with this value unless it's at least 5400 seconds (or 90 minutes). Is that incorrect?

name: Sentry
remoteRegistries:
pypi:
Expand Down
2 changes: 1 addition & 1 deletion airbyte-integrations/connectors/source-zendesk-support/metadata.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ data:
githubIssueLabel: source-zendesk-support
icon: zendesk-support.svg
license: ELv2
maxSecondsBetweenMessages: 10800
maxSecondsBetweenMessages: 60
name: Zendesk Support
remoteRegistries:
pypi:
Expand Down
Loading