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

Add init option to add raw response to return values #414

Merged
merged 5 commits into from
Jun 27, 2019

Conversation

nmuesch
Copy link
Contributor

@nmuesch nmuesch commented Jun 26, 2019

Reverts #412 and re implements in a different way.

This PR makes the approach threadsafe while keeping backwards compatibility by using a configuration option passed through the global init. return_raw_response is set to false by default, but when its explicitly enabled, we return the decoded response as well as the raw response object from the submit method.

@nmuesch nmuesch requested a review from a team as a code owner June 26, 2019 20:36
Copy link
Contributor

@gzussa gzussa left a comment

Choose a reason for hiding this comment

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

LGTM

@nmuesch nmuesch merged commit d72ad76 into master Jun 27, 2019
@nmuesch nmuesch deleted the nick/return_response branch June 27, 2019 18:47
@mzizzi
Copy link

mzizzi commented Jun 27, 2019

This is an improvement but it should be noted that the underlying response isn't always returned. The SDK's underlying HTTPClient could still cause exceptions to be thrown in some cases without returning the underlying response. e.g. some 4xx responses like 401 and all 5xx requests.

@mzizzi
Copy link

mzizzi commented Jun 27, 2019

Explicitly notifying @nmuesch again.

@nmuesch
Copy link
Contributor Author

nmuesch commented Jun 27, 2019

Hey @mzizzi Thats a valid point! I was taking a look but I don't see a way to keep this backwards compatible, thread safe, and return the raw response always. The HTTP Request wrapper, as it currently stands, wouldn't be able to return the response object and maintain the error codes its returning.

I'm going to reopen #408 but I don't think this is something we'll be able to tackle in the short term.

@VidhushiniSrinivasan16
Copy link

VidhushiniSrinivasan16 commented Oct 23, 2019

@nmuesch currently I get this issue :
ImportError: cannot import name '_return_raw_response' from 'datadog.api' (/var/task/datadog/api/init.py)
How do we enable return_raw_response?

@zippolyte
Copy link
Contributor

zippolyte commented Oct 23, 2019

Hey @VidhushiniSrinivasan16 ! Looks like your issue is similar to #460
You need to call initialize with the return_raw_response argument to enable it.

I recently merged a PR (#461) that would add its default value to the module in order to solve that import error, but it is not yet released.

Hope this helps !

@VidhushiniSrinivasan16
Copy link

@zippolyte Thanks Henry. That worked for me!

dabcoder pushed a commit to dabcoder/datadogpy that referenced this pull request Oct 25, 2019
* Revert "Add new top level `api` function to get the response object of last request (DataDog#412)"

* Add support for retrieving raw response
dabcoder pushed a commit to dabcoder/datadogpy that referenced this pull request Nov 28, 2019
* Revert "Add new top level `api` function to get the response object of last request (DataDog#412)"

* Add support for retrieving raw response
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants