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

🐛 Source Shopify: Fixed rate limits throttling #5470

Merged
merged 19 commits into from
Aug 24, 2021

Conversation

bazarnov
Copy link
Collaborator

@bazarnov bazarnov commented Aug 17, 2021

What

#5441 - Shopify Connector Rate Limiting
#5416 - Source shopify: confusing error message when rate limit is hit

How

  • added decorator balance_rate_limit method to control sleep time between each API call, by leveraging the "X-Shopify-Shop-Api-Call-Limit" header information, as described in Shopify Documentation
  • added unit tests to cover the issue
  • added note in Performance considerations section of shopify.md about rate limits.

Pre-merge Checklist

Expand the relevant checklist and delete the others.

Updating a connector

Community member or Airbyter

  • Unit & integration tests added and passing. Community members, please provide proof of success locally e.g: screenshot or copy-paste unit, integration, and acceptance test output. To run acceptance tests for a Python connector, follow instructions in the README. For java connectors run ./gradlew :airbyte-integrations:connectors:<name>:integrationTest.
  • Code reviews completed
  • Documentation updated
    • Changelog updated in docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
  • PR name follows PR naming conventions
  • Connector version bumped like described here

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

  • Create a non-forked branch based on this PR and test the below items on it
  • Build is successful
  • Credentials added to Github CI. Instructions.
  • /test connector=connectors/<name> command is passing.
  • New Connector version released on Dockerhub by running the /publish command described here

@bazarnov bazarnov self-assigned this Aug 17, 2021
@github-actions github-actions bot added the area/connectors Connector related issues label Aug 17, 2021
@github-actions github-actions bot added the area/documentation Improvements or additions to documentation label Aug 17, 2021
@bazarnov
Copy link
Collaborator Author

bazarnov commented Aug 17, 2021

/test connector=connectors/source-shopify

🕑 connectors/source-shopify https://github.com/airbytehq/airbyte/actions/runs/1140220772
✅ connectors/source-shopify https://github.com/airbytehq/airbyte/actions/runs/1140220772

@jrhizor jrhizor temporarily deployed to more-secrets August 17, 2021 17:29 Inactive
@bazarnov
Copy link
Collaborator Author

bazarnov commented Aug 18, 2021

/test connector=connectors/source-shopify

🕑 connectors/source-shopify https://github.com/airbytehq/airbyte/actions/runs/1143924329
✅ connectors/source-shopify https://github.com/airbytehq/airbyte/actions/runs/1143924329

@jrhizor jrhizor temporarily deployed to more-secrets August 18, 2021 16:41 Inactive
@bazarnov bazarnov requested a review from antixar August 18, 2021 16:54
@antixar antixar self-requested a review August 18, 2021 20:52
@bazarnov bazarnov requested a review from Phlair August 19, 2021 08:02
@bazarnov
Copy link
Collaborator Author

bazarnov commented Aug 19, 2021

/test connector=connectors/source-shopify

🕑 connectors/source-shopify https://github.com/airbytehq/airbyte/actions/runs/1146941752
✅ connectors/source-shopify https://github.com/airbytehq/airbyte/actions/runs/1146941752

@jrhizor jrhizor temporarily deployed to more-secrets August 19, 2021 12:31 Inactive
@bazarnov
Copy link
Collaborator Author

bazarnov commented Aug 20, 2021

/test connector=connectors/source-shopify

🕑 connectors/source-shopify https://github.com/airbytehq/airbyte/actions/runs/1150819644
✅ connectors/source-shopify https://github.com/airbytehq/airbyte/actions/runs/1150819644

@jrhizor jrhizor temporarily deployed to more-secrets August 20, 2021 13:24 Inactive
@bazarnov
Copy link
Collaborator Author

bazarnov commented Aug 20, 2021

/test connector=connectors/source-shopify

🕑 connectors/source-shopify https://github.com/airbytehq/airbyte/actions/runs/1151119628
✅ connectors/source-shopify https://github.com/airbytehq/airbyte/actions/runs/1151119628

@jrhizor jrhizor temporarily deployed to more-secrets August 20, 2021 15:03 Inactive
@bazarnov
Copy link
Collaborator Author

bazarnov commented Aug 23, 2021

/test connector=connectors/source-shopify

🕑 connectors/source-shopify https://github.com/airbytehq/airbyte/actions/runs/1158473499
✅ connectors/source-shopify https://github.com/airbytehq/airbyte/actions/runs/1158473499

@jrhizor jrhizor temporarily deployed to more-secrets August 23, 2021 11:37 Inactive
Copy link
Contributor

@Phlair Phlair left a comment

Choose a reason for hiding this comment

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

Like the decorator for throttling sleeps and thanks for adding unit tests.

Couple of comments and Qs :)

@bazarnov
Copy link
Collaborator Author

bazarnov commented Aug 23, 2021

/test connector=connectors/source-shopify

🕑 connectors/source-shopify https://github.com/airbytehq/airbyte/actions/runs/1160053552
✅ connectors/source-shopify https://github.com/airbytehq/airbyte/actions/runs/1160053552

@jrhizor jrhizor temporarily deployed to more-secrets August 23, 2021 20:05 Inactive
@rparrapy
Copy link
Contributor

FWIW, I am eagerly waiting for this to be merged :)

Copy link
Contributor

@Phlair Phlair left a comment

Choose a reason for hiding this comment

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

Thanks for making changes!
I'm approving now so we can get this released for users but for readability and structure sake, addressing the comment I left would be ideal. If not then at least making an issue to do so would be great (or let me know if you disagree!)

@bazarnov
Copy link
Collaborator Author

bazarnov commented Aug 24, 2021

/test connector=connectors/source-shopify

🕑 connectors/source-shopify https://github.com/airbytehq/airbyte/actions/runs/1162135031
✅ connectors/source-shopify https://github.com/airbytehq/airbyte/actions/runs/1162135031

@jrhizor jrhizor temporarily deployed to more-secrets August 24, 2021 10:11 Inactive
@bazarnov
Copy link
Collaborator Author

bazarnov commented Aug 24, 2021

/publish connector=connectors/source-shopify

🕑 connectors/source-shopify https://github.com/airbytehq/airbyte/actions/runs/1162176109
✅ connectors/source-shopify https://github.com/airbytehq/airbyte/actions/runs/1162176109

@jrhizor jrhizor temporarily deployed to more-secrets August 24, 2021 10:25 Inactive
@bazarnov bazarnov merged commit f76a135 into master Aug 24, 2021
@bazarnov bazarnov deleted the bazarnov/shopify-rate-limits-issue branch August 24, 2021 11:39
avida pushed a commit that referenced this pull request Aug 25, 2021
#5416 - source shopify: confusing error message when rate limit is hit
#5441 - Shopify Connector Rate Limiting
PR: #5470 - #5470

Co-authored-by: Oleksandr Bazarnov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/connectors Connector related issues area/documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Shopify Connector Rate Limiting source shopify: confusing error message when rate limit is hit
6 participants