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

The JSON adapter should return an empty response on 204 rather than the response object #797

Open
briantist opened this issue Mar 2, 2022 · 1 comment · May be fixed by #898
Open

The JSON adapter should return an empty response on 204 rather than the response object #797

briantist opened this issue Mar 2, 2022 · 1 comment · May be fixed by #898
Assignees
Labels
adapters related to the Adapter system

Comments

@briantist
Copy link
Contributor

briantist commented Mar 2, 2022

hvac/hvac/adapters.py

Lines 365 to 371 in ec048de

if response.status_code == 200:
try:
return response.json()
except ValueError:
pass
return response

On a 204 response, the entire response object is returned, but I think it should be an empty dict instead (or maybe None would be fine as well), to be more consistent, since Vault is known to return 204 responses when it has nothing to say, and that result is more appropriate to deal with as a client.

As an example, if I want to test for this case, I need to import requests and test the the type of the response, or I can try checking for a status_code field, but if the vault request returned some data with a status_code field, then I can't rely on that.

There's a number of workarounds I can think of to handle this, but it'd be better for this common response type to be handled by the adapter in a way that's friendlier to deal with on the client side.

@briantist
Copy link
Contributor Author

@briantist briantist linked a pull request Sep 18, 2022 that will close this issue
@briantist briantist self-assigned this Sep 17, 2023
@briantist briantist added the adapters related to the Adapter system label Sep 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
adapters related to the Adapter system
Projects
None yet
1 participant