-
Notifications
You must be signed in to change notification settings - Fork 258
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
Client.request_and_return no longer works with NoneResponse #840
Comments
Another approach to this is to change the Client.parse_response to handle the NoneResponse specially (acceptable for only_extras() to be true, and also ok for resp to evaluate to False). I dislike NoneResponse being special, but this also lets us still run the store_response and resp.verify. |
Can you elaborate for which usecase you need the old behaviour please? I see the issue, yes, the return type changed if you do not care for the response message and just want the response HTTP code. The only internal users of that functionality got changed to just handle the case directly, because it ends up being just http_request() + some minimal http status code handling. Seems i missed the docstring of the request_and_return() method that spells out the special handling for no expected responses. If there is some convincing usecase for this usage pattern, we might simply add a convenience method that does it, but currently i do not see a real benefit over just calling http_request() directly and simply reading the status code yourself. |
For my use case, I have a user management endpoint to which I submit a POST to delete a user, and get back an empty response. I can easily work around this by passing None as the response type, so it's not a real problem for me. It's easy to imagine an expected response that could be empty, but could also have fields where you want to take advantage of the verification when all the fields are optional, or storing the response, but I don't actually need that functionality myself. Feel free to close this, if you don't think it needs to be handled by the library. |
Ok, thank you. I'll have to think about it a bit. The mix of Message and http status code as return type kind of makes the type annotations for that code really ugly and hard to reason about. |
I did appreciate having the types cleaned up so much. Overall it's much nicer now, but I did want you to know about the compatibility issue with code that worked with 1.4. I'll use the workaround in my calling code for now. |
In 1.5.0, client.request_and_return doesn't work with NoneResponse anymore.
As a workaround, you can pass None as the response, but that prevents more generic protocol registration from working.
This broke in PR 817, where
src/oic/oauth2/__init__.py
lost the handling of the empty body on line 752 where it returned the status code instead.The text was updated successfully, but these errors were encountered: