-
Notifications
You must be signed in to change notification settings - Fork 1k
Fail to upload package when description rendering is invalid. #3980
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
Conversation
This branch prevents a package upload if the rendered content for the description_content_type is invalid. If no description_content_type is set, 'text/x-rst' is assumed as the type. Fixes pypi#3966
warehouse/forklift/legacy.py
Outdated
"See {projecthelp} " | ||
"for more information.").format( | ||
description_content_type=description_content_type, | ||
projecthelp=request.help_url(_anchor='project-name'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll want to update this line to point to a new section of the help page (https://github.com/pypa/warehouse/blob/master/warehouse/templates/pages/help.html) and also add that section.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any particular person who deals with docs specifically that I should ping or should I take a crack at the documentation myself?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope! Feel free to do this yourself in this PR.
warehouse/forklift/legacy.py
Outdated
raise _exc_with_message( | ||
HTTPBadRequest, | ||
("The description failed to render " | ||
"for '{description_content_type}'. " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be confusing to users if they haven't explicitly set a description-content-type
but they're seeing text/x-rst
in the error message. We might want to tweak this so that it's more general, or give a different error message when the content type hasn't been specified, and it's not valid rST.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure thing. I can do that.
@di Please let me know if you need any more for the docs or the error message. I did my best, however, PyPI might have some guidelines or voice that I'm unaware of, and I can tweak the wording until it feels right. |
Thanks for this, it looks good to me! If there's additional tweaks to the docs warning we can follow up in additional PRs. Thanks! |
Woohoo! Thanks for the help @di! I'm glad I was able to fix a PyPI issue that has bugged me for ages. |
This branch prevents a package upload if the rendered content for the
description_content_type is invalid. If no description_content_type is
set, 'text/x-rst' is assumed as the type.
I'm sure the error message could point to something more informative that explains why failed rendering blocks an upload. Since such a page doesn't exist for now, I went with a somewhat generic error message. I'm happy to push more commits when a specific thing is in place. I'm a bit ignorant about how docs are handled for warehouse.
Fixes #3966