Skip to content

Ensure all WSGITransport environs have a SERVER_PROTOCOL#2708

Merged
lovelydinosaur merged 1 commit intoencode:masterfrom
lazorchakp:wsgi-server-protocol
May 19, 2023
Merged

Ensure all WSGITransport environs have a SERVER_PROTOCOL#2708
lovelydinosaur merged 1 commit intoencode:masterfrom
lazorchakp:wsgi-server-protocol

Conversation

@lazorchakp
Copy link
Contributor

@lazorchakp lazorchakp commented May 18, 2023 •
edited
Loading

Summary

  • I noticed that WSGI environs sent to applications with httpx.Client(app=app) did not contain a SERVER_PROTOCOL
  • PEP-3333 indicates that this key must be present in the WSGI environ, and popular WSGI frameworks (such as webob) assume it will be as well
  • It looks like httpx defaults to using HTTP/1.1 for a nearly identical case in the ASGITransport, so I did the same here
  • I'm more than happy to discuss any of this further!

Checklist

  • I understand that this PR may be closed in case there was no previous discussion. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the documentation accordingly.
    • to my knowledge, this does not require a documentation update

@lazorchakp lazorchakp marked this pull request as ready for review May 18, 2023 20:51
zanieb
zanieb approved these changes May 18, 2023
View reviewed changes
Copy link
Contributor

@zanieb zanieb left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for contributing.

Another maintainer may weigh in but it seems unlikely that this was intentionally excluded.

@lazorchakp
Copy link
Contributor Author

lazorchakp commented May 18, 2023

Great, thanks! I'll give it a day or so before merging to see if anyone has other thoughts

@lovelydinosaur
Copy link
Contributor

lovelydinosaur commented May 19, 2023

valid.

@lovelydinosaur lovelydinosaur merged commit abb994c into encode:master May 19, 2023
@lovelydinosaur
Copy link
Contributor

lovelydinosaur commented May 19, 2023

Process improvements we can make here...

  • @tomchristie - Thanks for the quick merge, though you don't need to always play that role. If you could allow other maintainers more space here too that'd probs be good for the team as a whole.
  • @encode/maintainers - We ought to start making sure we include CHANGELOG changes as part of functional or bugfix PRs. That'll help ensure that we're properly describing the change from a user perspective, and will also make the release process simpler and less error-prone. We should probably update the pull request template to include this as an item?
  • @lazorchakp would you consider opening a PR to add this to the CHANGELOG under a new "development" heading?

@Kludex
Copy link
Contributor

Kludex commented May 19, 2023

Yes. 👍

@lazorchakp lazorchakp deleted the wsgi-server-protocol branch May 19, 2023 14:12
@lazorchakp lazorchakp mentioned this pull request May 19, 2023
Merged
3 tasks
@trim21 trim21 mentioned this pull request Aug 1, 2023
Merged
@zevisert zevisert mentioned this pull request Jan 4, 2024
Closed
samclearman pushed a commit to titanmsp/httpx that referenced this pull request Apr 26, 2025
@lazorchakp @samclearman
Ensure all WSGITransport environs have a SERVER_PROTOCOL (encode#2708)
6f7882f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zanieb zanieb zanieb approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@lazorchakp @lovelydinosaur @Kludex @zanieb