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

[airbyte-cdk] - Update HttpClient to call authenticator on backoff retry attempts in HttpClient._send #47191

Merged
merged 4 commits into from
Oct 23, 2024

Conversation

pnilan
Copy link
Contributor

@pnilan pnilan commented Oct 21, 2024

What

  • OC: https://github.com/airbytehq/oncall/issues/6675
  • The backoff handler wraps the HttpClient._send method. When the source-google-analytics-data-api would exceed its 1-hour quota threshold it would back off for 1800s (30 mins) twice. On the third attempt, after 60 minutes from the original attempt, the _send method is retried again, but does not invoke the authenticator's __call__ method, resulting in an attempt w/o refreshing the access token. This is likely an issue for all connectors that use oauth and have to backoff for a duration longer than the lifespan of the access token.
  • Reduces complexity of _send by moving error resolution handling to new _handle_error_resolution method. (Required to due change exceeding flake8 complexity threshold)

How

  • The HttpClient._send method already included logic for checking the attempt count. On subsequent attempts, will re-invoke the authenticator w/ the given request. For oauth authenticators, this will typically check the expiration of the token and refresh if needed.

Review guide

  1. http_client.py

User Impact

  • Improves sync reliability for connectors w/ oauth

Can this PR be safely reverted and rolled back?

  • YES 💚
  • NO ❌

Copy link

vercel bot commented Oct 21, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
airbyte-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 21, 2024 11:49pm

@octavia-squidington-iii octavia-squidington-iii added the CDK Connector Development Kit label Oct 21, 2024
@pnilan pnilan requested review from a team, aldogonzalez8 and aaronsteers October 21, 2024 19:36
Copy link
Contributor

@aldogonzalez8 aldogonzalez8 left a comment

Choose a reason for hiding this comment

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

APPROVED

@pnilan
Copy link
Contributor Author

pnilan commented Oct 22, 2024

/approve-regression-tests https://github.com/airbytehq/airbyte/actions/runs/11468980387/job/31915233015

Check job output.

✅ Approving regression tests

@pnilan pnilan merged commit 15da7f2 into master Oct 23, 2024
45 of 52 checks passed
@pnilan pnilan deleted the pnilan/p1-fix-backoff-retry-with-auth branch October 23, 2024 00:05
Copy link

sentry-io bot commented Oct 23, 2024

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ airbyte_cdk.sources.streams.http.exceptions.UserDefinedBackoffException: Internal server error. /usr/local/lib/python3.9/site-packages/airbyte_... View Issue

Did you find this useful? React with a 👍 or 👎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CDK Connector Development Kit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants