-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
🐛 source mixpanel: match API limitation of requests rate #7439
Conversation
…e because it does not take into consideration actual number of requests made in previous and next streams.
/test connector=connectors/source-mixpanel
|
/test connector=connectors/source-mixpanel
|
/test connector=connectors/source-mixpanel
|
Please increase the |
/test connector=connectors/source-mixpanel
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Several minor comments
airbyte-integrations/connectors/source-mixpanel/source_mixpanel/source.py
Show resolved
Hide resolved
airbyte-integrations/connectors/source-mixpanel/source_mixpanel/source.py
Show resolved
Hide resolved
airbyte-integrations/connectors/source-mixpanel/source_mixpanel/source.py
Show resolved
Hide resolved
/test connector=connectors/source-mixpanel
|
/test connector=connectors/source-mixpanel
|
/test connector=connectors/source-mixpanel
|
thanks @midavadim can we ship this? 🚀 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
This is the evident hotfix and the current reviews are enough. |
/publish connector=connectors/source-mixpanel
|
…-requests' into midavadim/7126-mixpanel-too-many-requests # Conflicts: # docs/integrations/sources/mixpanel.md
* Added delay to for all streams, removed logic which increase reqs rate because it does not take into consideration actual number of requests made in previous and next streams. * Fixed argmument passing * Increased timeout for SAT * Increased timeout for SAT * bump version * bumped connector version, updated change log Co-authored-by: Marcos Marx <marcosmarxm@gmail.com>
) * Added delay to for all streams, removed logic which increase reqs rate because it does not take into consideration actual number of requests made in previous and next streams. * Fixed argmument passing * Increased timeout for SAT * Increased timeout for SAT * bump version * bumped connector version, updated change log Co-authored-by: Marcos Marx <marcosmarxm@gmail.com>
What
From logs:
2021-10-17 16:40:14 INFO Using start_date: 2020-10-17, end_date: 2021-10-17
2021-10-17 16:40:14 INFO Read 12 records from annotations stream
2021-10-17 16:40:59 INFO Read 57478 records from cohort_members stream
2021-10-17 16:41:09 INFO Read 17 records from cohorts stream
2021-10-17 16:43:28 INFO Read 52921 records from engage stream
2021-10-17 17:40:32 INFO Read 5930103 records from export stream
2021-10-17 17:44:11 INFO Records read: 6051000
2021-10-17 17:46:49 ERROR Stream funnels: 429 Too Many Requests - {"request": "/api/2.0/funnels?from_date=2021-04-19&to_date=2021-05-15&funnel_id=3069709&unit=day", "error": "Query rate limit exceeded for project_id: 876089. 1/5 queries running concurrently and 401/400 queries running in the last hour. For more information, please consult our documentation at https://help.mixpanel.com/hc/en-us/articles/115004602563-Rate-Limits-for-Export-API-Endpoints"}
Streams order:
Annotations(authenticator=auth, **config),
Cohorts(authenticator=auth, **config),
CohortMembers(authenticator=auth, **config),
Engage(authenticator=auth, **config),
Export(authenticator=auth, **config),
Funnels(authenticator=auth, **config),
Revenue(authenticator=auth, **config),
So connector was able to read 6051000 records during the first hour and then on 'funnels', it hits API rate limit.
Code review showed that 'funnels' stream did not have limitation for query rate.
How
Added delay to for all streams, removed logic which increases reqs rate because it does not take into consideration actual number of requests made in previous and next streams.
Recommended reading order
x.java
y.python
Pre-merge Checklist
Expand the relevant checklist and delete the others.
New Connector
Community member or Airbyter
airbyte_secret
./gradlew :airbyte-integrations:connectors:<name>:integrationTest
.README.md
bootstrap.md
. See description and examplesdocs/SUMMARY.md
docs/integrations/<source or destination>/<name>.md
including changelog. See changelog exampledocs/integrations/README.md
airbyte-integrations/builds.md
Airbyter
If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.
/test connector=connectors/<name>
command is passing./publish
command described hereUpdating a connector
Community member or Airbyter
airbyte_secret
./gradlew :airbyte-integrations:connectors:<name>:integrationTest
.README.md
bootstrap.md
. See description and examplesdocs/integrations/<source or destination>/<name>.md
including changelog. See changelog exampleAirbyter
If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.
/test connector=connectors/<name>
command is passing./publish
command described hereConnector Generator
-scaffold
in their name) have been updated with the latest scaffold by running./gradlew :airbyte-integrations:connector-templates:generator:testScaffoldTemplates
then checking in your changes