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

show full HTTP response on failure (Feat/1605) #1895

Open
wants to merge 9 commits into
base: devel
Choose a base branch
from

Conversation

willi-mueller
Copy link
Collaborator

@willi-mueller willi-mueller commented Sep 28, 2024

Description

This PR implements error logging of the full HTTP response on error when the dlt.sources.helpers.requests.Session is used.

Unless a custom session with different configuration is used, all HTTP responses with an error code 4xx will log an error.

This includes:

  1. the RESTClient
  2. each call made by the dlt.sources.rest_api

Related Issues

@willi-mueller willi-mueller linked an issue Sep 28, 2024 that may be closed by this pull request
Copy link

netlify bot commented Sep 28, 2024

Deploy Preview for dlt-hub-docs ready!

Name Link
🔨 Latest commit 9e066d8
🔍 Latest deploy log https://app.netlify.com/sites/dlt-hub-docs/deploys/6734c5996d93be0008e82824
😎 Deploy Preview https://deploy-preview-1895--dlt-hub-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@willi-mueller willi-mueller changed the title Feat/1605 rest client show full response on failure show full HTTP response on failure (Feat/1605) Sep 28, 2024
@willi-mueller willi-mueller self-assigned this Sep 28, 2024
@burnash
Copy link
Collaborator

burnash commented Nov 11, 2024

I propose an alternative solution to logging the full HTTP response on error: we can use a context manager catch_http_error that captures HTTP errors and allows for flexible and controlled error handling / debugging.

Example:

import dlt
from dlt.sources.helpers.rest_client import paginate, catch_http_error


@dlt.resource(
    table_name="issues",
    write_disposition="replace",
    primary_key="id",
)
def get_issues():
    for page in paginate(
        "https://api.github.com/repos/dlt-hub/dlt/issues",
        headers={"Authorization": "Bearer invalid_token_here"},
    ):
        yield page


pipeline = dlt.pipeline(
    pipeline_name="github_issues_merge",
    destination="duckdb",
)

with catch_http_error() as http_error:
    load_info = pipeline.run(get_issues)

if http_error:
    print(f"HTTP {http_error.response.status_code}: {http_error.response.content}")

Pros:

  • improved flexibility: the use can interact with captured HTTPError
  • cleaner and safer logs
  • selective usage: no need to alter the session

The rationale is that in my opinion accessing the HTTPError should be done on the higher level of abstraction than Requests's session as when the user is using the request/response directly they already have easy access to HTTP errors.
So the idea of this change is to provide some way to extract HTTPError from the dlt exceptions.

@burnash burnash requested a review from rudolfix November 11, 2024 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

show full response when printing failed api calls
2 participants