Skip to content
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

Document the project and file upload limits in pypi on the FAQ page #14478

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

SurenNihalani
Copy link

@SurenNihalani SurenNihalani commented Sep 2, 2023

This is for #6192 . I had to create a new constants module because import warehouse.legacy leads to circular imports. Instead of resolving that, I went with creating a new file

Test Plan:
make build, make initdb, make serve and read http://localhost/help/#project-size-limit .

This is how it looks like:
image

Fixes #6192.

@SurenNihalani SurenNihalani requested a review from a team as a code owner September 2, 2023 03:37
@di
Copy link
Member

di commented Sep 5, 2023

From #6192 (comment):

I'd prefer using the variables to control the output so as to prevent drift between the values and the docs.

I think I agree, these variables can be found here:

MAX_FILESIZE = 100 * ONE_MB
MAX_SIGSIZE = 8 * 1024
MAX_PROJECT_SIZE = 10 * ONE_GB

@miketheman miketheman added the awaiting-response PRs and issues that are awaiting author response label Sep 5, 2023
@SurenNihalani SurenNihalani force-pushed the origin_6192 branch 2 times, most recently from c81adea to bfb451f Compare September 13, 2023 21:58
@SurenNihalani
Copy link
Author

@di , updated the PR. Thanks for the suggestion. Please review if you have cycles

@SurenNihalani
Copy link
Author

Does anyone know why this is awaiting response? This should be ready for review

Comment on lines +151 to +154
settings["warehouse.forklift.legacy.MAX_FILESIZE_MB"] = MAX_FILESIZE / ONE_MB
settings["warehouse.forklift.legacy.MAX_PROJECT_SIZE_GB"] = (
MAX_PROJECT_SIZE / ONE_GB
)
Copy link
Member

Choose a reason for hiding this comment

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

We should make sure that this is the source of truth to these values everywhere, so instead of using MAX_FILESIZE and MAX_PROJECT_SIZE outside of this file, we should use the values from settings.

@di
Copy link
Member

di commented Apr 22, 2024

Hi @SurenNihalani, are you able to resolve the conflicts here and address the review so we can merge this? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-response PRs and issues that are awaiting author response
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Document the package size limit
3 participants