Skip to content

bpo-46285: Add command-line option -p/--protocol to module http.server - #30999

Merged
JelleZijlstra merged 5 commits into
python:mainfrom
geryogam:patch-26
May 2, 2022
Merged

bpo-46285: Add command-line option -p/--protocol to module http.server#30999
JelleZijlstra merged 5 commits into
python:mainfrom
geryogam:patch-26

Conversation

@geryogam

@geryogamgeryogam commented Jan 28, 2022•
edited
Loading

Copy link
Copy Markdown
Contributor

This PR will add command-line option -p/--protocol to module http.server which specifies the HTTP version to which the server is conformant (HTTP/1.1 servers can now be run from the command-line interface of module http.server):

python -m http.server --protocol HTTP/1.1

https://bugs.python.org/issue46285

@geryogamgeryogam mentioned this pull request Jan 29, 2022
Merged
@merwok

merwok commented Jan 29, 2022

Copy link
Copy Markdown
Member

Thanks for the the PR! Let’s merge this one after the bug fix to the same module.

@geryogam
geryogamforce-pushed the patch-26 branch 3 times, most recently from 5323621 to f5e0dc4CompareJanuary 30, 2022 23:38
@merwokmerwok self-assigned this Jan 31, 2022
merwok
merwok reviewed Feb 3, 2022
View reviewed changes
Comment threadDoc/library/http.server.rst Outdated
@merwok

merwok commented Feb 4, 2022

Copy link
Copy Markdown
Member

Please avoid rebases and force-pushes for CPython, github does not have a good experience for reviewers.
Regular merges are also easier to do!

merwok
merwok reviewed Feb 4, 2022
View reviewed changes
Comment threadDoc/library/http.server.rst
@merwok
merwok requested a review from orsenthilMay 1, 2022 12:24
orsenthil
orsenthil approved these changes May 2, 2022
View reviewed changes

@orsenthilorsenthil left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM.

I had a brief hesitancy for exposing this protocol version via flag as the support is not comprehensive. I am pretty sure we are missing many features.

But given that class supports .protocol_version as a attribute, exposing it via a command line flag is alright.

@JelleZijlstraJelleZijlstra self-assigned this May 2, 2022
@JelleZijlstra
JelleZijlstra merged commit 2d30ade into python:mainMay 2, 2022
@geryogam

geryogam commented May 3, 2022

Copy link
Copy Markdown
ContributorAuthor

Thanks @merwok, @JelleZijlstra, and @orsenthil for the review!

@geryogam
geryogam deleted the patch-26 branch May 3, 2022 05:58
@merwokmerwok mentioned this pull request May 3, 2022
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@merwokmerwokmerwok left review comments
@orsenthilorsenthilorsenthil approved these changes
@JelleZijlstraJelleZijlstraJelleZijlstra approved these changes

Assignees

@merwokmerwok

@JelleZijlstraJelleZijlstra

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@geryogam@merwok@orsenthil@JelleZijlstra@the-knights-who-say-ni@bedevere-bot