-
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 Salesforce: Add retry on REST API #36885
🐛 Source Salesforce: Add retry on REST API #36885
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
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.
LGTM
@@ -0,0 +1,39 @@ | |||
# Copyright (c) 2024 Airbyte, Inc., all rights reserved. | |||
|
|||
from datetime import datetime |
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.
This will clash with the source-salesforce part of https://github.com/airbytehq/airbyte/pull/36811/files#diff-82cd2ba6c00f795c829bdc86d1f039933a6168b6c1c6c1a4b89226c7b5c6bf67 but that shouldn't be too much of a hassle
We have added retries on 406 on the https://github.com/airbytehq/airbyte/blob/4ac078b3826891ca33bbe394bdaa9891f0a28bf9/airbyte-integrations/connectors/source-salesforce/source_salesforce/rate_limiting.py#L30-L32. We are however still seeing syncs fail because of that. Checking all the cases on April 5th and 6th, there all occur in
_fetch_next_page_for_chunk
. Hence, I'll blindly add retry there.This can be safely reverted