-
Notifications
You must be signed in to change notification settings - Fork 1k
Advise packagers on requesting upload size limit increase #3175
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
warehouse/forklift/legacy.py
Outdated
limit=file_size_limit // (1024 * 1024), | ||
)) | ||
limit=file_size_limit // (1024 * 1024)) + | ||
"See https://pypi.org/help/#file_size_limit", |
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.
Instead of hardcoding this link, could you generate it like request.route_url('help', _anchor='file-file-size-limit')
, otherwise it will be wrong for TestPyPI.
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.
(Also I realize there are a few other places where this isn't being done... perhaps you'd like to fix those too? 🙂)
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.
AHA. I tried to figure that out - evidently not hard enough. :) Thanks for the tip. Fixing.
warehouse/templates/pages/help.html
Outdated
@@ -152,6 +154,17 @@ <h2>{{ project_name_claim() }}</h2> | |||
</p> | |||
</section> | |||
|
|||
<section id="file_size_limit" class="common-question"> |
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.
I think our frontend generally prefers kebab-case
for ids rather than snake_case
.
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.
Got it. Fixing.
warehouse/templates/pages/help.html
Outdated
If you can't upload your release to PyPI because you're hitting the upload file size limit, please <a href="https://github.com/pypa/warehouse/issues/new">file an issue</a> and tell us:</p> | ||
<ul> | ||
<li>The name of the project</li> | ||
<li>The size of your release (example: 300 MB)</li> |
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.
300MB is kind of on the larger end of our normal requests. Instead of providing an example we could just say:
The size of your release, in megabytes
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.
Got it. Fixing.
warehouse/templates/pages/help.html
Outdated
<ul> | ||
<li>The name of the project</li> | ||
<li>The size of your release (example: 300 MB)</li> | ||
<li>Which index/indexes you need the increase for (PyPI, Test PyPI, or both)</li> |
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's probably also worth adding somewhere (perhaps here) that if they are trying to upload the first release for their project, and that release is larger than the default limit, they need to make an initial release that is smaller than the default limit before we are able to increase the limit for the given 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.
Thanks. Fixed.
Failing a test because I am not properly plumbing the route call through Pyramid's URL mapping:
Looks like we should also fix https://github.com/pypa/warehouse/blob/master/warehouse/forklift/legacy.py#L833 to use URL mapping. Going to fix after lunch, would welcome tips. |
Yes, you'll need to mock out db_request.route_url = pretend.call_recorder(
lambda route, **kw: "/the/help/url/"
) |
@di Thanks for the tip. I have fixed the test for the filesize message, and will look at the other upload error messages that need URL substitution in a separate PR. I believe this is ready to merge. |
* Advise packagers on requesting upload size limit increase * Add filesize limit help link to error message * Fix typo in comment
No description provided.