Skip to content

Conversation

@jcomeauictx
Copy link

@jcomeauictx jcomeauictx commented Dec 25, 2023

@ghost
Copy link

ghost commented Dec 25, 2023

All commit authors signed the Contributor License Agreement.
CLA signed

@bedevere-app
Copy link

bedevere-app bot commented Dec 25, 2023

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

This change need tests.

I am not sure how much useful is this feature. It only allows to override the MIME type of unrecognized types. And would not it be simpler to configure your browser?

Copy link
Member

@encukou encukou left a comment

Choose a reason for hiding this comment

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

The feature looks useful to me; I've wanted it once or twice before. However, adding it would be a lot work. Not sure if it's worth your time.

The change needs documentation (in Doc/library/http.server.rst, both for the attribute and CLI option).

@donBarbos
Copy link
Contributor

@jcomeauictx are you going to continue working on this PR?

@jcomeauictx
Copy link
Author

@jcomeauictx are you going to continue working on this PR?

I've forgotten what else needs to be done. And in any case, nobody else seems interested in it.


python -m http.server --default-content-type text/html

.. versionchanged:: 3.13
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
.. versionchanged:: 3.13
.. versionchanged:: next

@donBarbos
Copy link
Contributor

I've forgotten what else needs to be done. And in any case, nobody else seems interested in it.

I'm interested :)
I think if you add tests you can ping someone from core members

@jcomeauictx
Copy link
Author

OK, I'll look into it tomorrow. No idea where to add them, though, or what precisely needs to be tested.

@donBarbos
Copy link
Contributor

donBarbos commented Mar 8, 2025

we have at least two test classes where default_request_version attribute are redefined (file test_httpservers.py lines 93 and 321) and then checked, I think we need to do the same with our new attribute

@jcomeauictx
Copy link
Author

I've looked at test_httpservers.py but really don't know how to go about it. Would you be willing to write the tests?

@donBarbos
Copy link
Contributor

I've looked at test_httpservers.py but really don't know how to go about it. Would you be willing to write the tests?

yea, thank you. I will try to send suggestion as soon as possible.
but if you send it yourself earlier I won't be against :-)

@donBarbos
Copy link
Contributor

@jcomeauictx could you merge main branch for correct tests?

@jcomeauictx
Copy link
Author

done, I think? also changed versionchanged to next as per your suggestion above.

@donBarbos
Copy link
Contributor

@jcomeauictx could you merge main branch again and update your new option description? (because we updated CLI documentation style)

@donBarbos
Copy link
Contributor

cc @picnixz what do you think about this feature?

Copy link
Member

@picnixz picnixz left a comment

Choose a reason for hiding this comment

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

I won't comment of the usefulness as I don't think I'll ever use it but the feature seems easy to implement so I'll accept the work. However, we need:

  • tests
  • a What's New entry (in Doc/whatsnew/3.14.rst; locate the http.server section in Improved Modules, or create one if none exists and carefully write it in the same style as other sections do)

@jcomeauictx
Copy link
Author

donBarbos, I just added it, but isn't the indentation wrong?

@jcomeauictx
Copy link
Author

jcomeauictx commented Mar 16, 2025 via email

@donBarbos
Copy link
Contributor

Sorry I can't. I mean move on to the rest options

@jcomeauictx
Copy link
Author

jcomeauictx commented Mar 16, 2025 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants