-
Notifications
You must be signed in to change notification settings - Fork 186
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 response when printing failed api calls #1605
Comments
Worth pointing out that there are potential security concerns with printing a full response on error. However it looks like certain methods already do that... |
Hey @shuaahvee thanks for suggestion. It's very important for us to let users know why the API call failed. |
rest_api_source! |
Hey @shuaahvee , thank you for your feature request! We have implemented it here: #1895 I'd be happy to hear your feedback! |
@shuaahvee thanks again for reporting this issue. And also for noticing that leaks full response in some places – we're going to address that in a separate PR. @willi-mueller @shuaahvee I propose a different solution than logging: #1895 (comment) |
Feature description
If an API call fails, the cli calls the requests library's raise_for_status method which prints the request's
status code
andreason
. But not all APIs use the response's reason attribute. It would be helpful if we could instead either configure what response attributes get printed or at least printresponse.text
. E.g. I was debugging a klaviyo connector and got the usual response message when my api call failed. The cause was that I hadn't configured the request header correctly but the only way I figured that out was by editing the raise_for_status method's code to look like this:if 400 <= self.status_code < 500: http_error_msg = ( f"{self.status_code} Client Error: {reason} for url: {self.url}. Full response: {self.text}" )
Are you a dlt user?
Yes, I'm already a dlt user.
Use case
debugging failed klaviyo api calls
Proposed solution
override the raise_for_status method? or maybe add a config that lets you specify which response attributes should be printed on failure
Related issues
No response
The text was updated successfully, but these errors were encountered: