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: Log http status code and content in backoff handlers #8829

Merged
merged 4 commits into from
Dec 16, 2021

Conversation

augan-rymkhan
Copy link
Contributor

@augan-rymkhan augan-rymkhan commented Dec 16, 2021

What

https://github.com/airbytehq/airbyte/blob/master/airbyte-cdk/python/airbyte_cdk/sources/streams/http/http.py#L227
When backoff is happened we don't know the response status code and its content (usefull message about why request was rejected from host). We only know it is 429 or 500 <= status code 600

How

Log http response status code and content in log_retry_attempt inside default_backoff_handler and
in sleep_on_ratelimit inside user_defined_backoff_handler

Recommended reading order

airbyte-cdk/python/airbyte_cdk/sources/streams/http/rate_limiting.py

@github-actions github-actions bot added the CDK Connector Development Kit label Dec 16, 2021
@augan-rymkhan augan-rymkhan temporarily deployed to more-secrets December 16, 2021 04:41 Inactive
@augan-rymkhan augan-rymkhan temporarily deployed to more-secrets December 16, 2021 05:52 Inactive
@augan-rymkhan augan-rymkhan changed the title Log http status code and content in default backoff handler Airbyte CDK: Log http status code and content in default backoff handler Dec 16, 2021
@augan-rymkhan augan-rymkhan changed the title Airbyte CDK: Log http status code and content in default backoff handler Airbyte CDK: Log http status code and content in backoff handlers Dec 16, 2021
@augan-rymkhan augan-rymkhan temporarily deployed to more-secrets December 16, 2021 06:07 Inactive
@@ -21,6 +21,8 @@
def default_backoff_handler(max_tries: int, factor: int, **kwargs):
def log_retry_attempt(details):
_, exc, _ = sys.exc_info()
if exc.response is not None:
logger.info(f"Status code: {exc.response.status_code}, Response Content: {exc.response.content}")
Copy link
Contributor

Choose a reason for hiding this comment

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

According to your following code, you could use just if exc.response: and it would be clear enough.

@augan-rymkhan augan-rymkhan temporarily deployed to more-secrets December 16, 2021 13:43 Inactive
@augan-rymkhan
Copy link
Contributor Author

augan-rymkhan commented Dec 16, 2021

/publish-cdk dry-run=false

🕑 https://github.com/airbytehq/airbyte/actions/runs/1587747875
https://github.com/airbytehq/airbyte/actions/runs/1587747875

@augan-rymkhan augan-rymkhan merged commit 7fbdfc6 into master Dec 16, 2021
@augan-rymkhan augan-rymkhan deleted the arymkhan/log-http-response-status-and-content branch December 16, 2021 14:20
schlattk pushed a commit to schlattk/airbyte that referenced this pull request Jan 4, 2022
…rbytehq#8829)

* Log http status code and content in default backoff handler

* Log http status code and content in usef defined backoff handler

* updated cdk version and changelog

* make it clear: exc.response

Co-authored-by: auganbay <auganenu@gmail.com>
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