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

Add separate docs tarball limits and support infinity #143

Merged
merged 5 commits into from
Feb 29, 2024

Conversation

wojtekmach
Copy link
Member

Closes #136
Closes #137

Before this patch we had a single tarball limit i.e. used for package and docs tarballs. The original issue is about bumping docs tarball limits so I'd focus on just that and wait for concrete uses cases for bumping the package tarball limit.

This additionally avoids the issue of someone publishing a bigger package tarball and users not being able to unpack it until they are on latest Hex (which would have bumped the limit).

WDYT?

@wojtekmach wojtekmach requested a review from ericmj February 29, 2024 09:52
@ericmj
Copy link
Member

ericmj commented Feb 29, 2024

I like the idea of separating the package and docs settings. But I think it's time to bump package sizes as well since we've had a few requests for it.

Can we also have the option to skip the limits if the field is set to undefined? Clients should set this when unpacking so they don't break on packages that use the new limits before updating.

@wojtekmach wojtekmach changed the title Add separate docs tarball limits Add separate docs tarball limits and support undefined Feb 29, 2024
@wojtekmach
Copy link
Member Author

Good call, updated!

@@ -601,6 +616,12 @@ gzip_no_header(Uncompressed) ->
%% Helpers
%%====================================================================

%% @private
valid_size(Binary, undefined) when is_binary(Binary) ->
Copy link
Member Author

Choose a reason for hiding this comment

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

another idea is to call the sentinel value infinity:

Suggested change
valid_size(Binary, undefined) when is_binary(Binary) ->
valid_size(Binary, infinity) when is_binary(Binary) ->

but perhaps undefined is more often used for things like this.

Copy link
Member

Choose a reason for hiding this comment

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

I like infinity more 👍

@wojtekmach
Copy link
Member Author

@ericmj okay, let's bump package tarball sizes too. Same limit for both, 16 MiB compressed & 128 MiB uncompressed?

@@ -601,6 +616,12 @@ gzip_no_header(Uncompressed) ->
%% Helpers
%%====================================================================

%% @private
valid_size(Binary, undefined) when is_binary(Binary) ->
Copy link
Member

Choose a reason for hiding this comment

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

I like infinity more 👍

@wojtekmach wojtekmach changed the title Add separate docs tarball limits and support undefined Add separate docs tarball limits and support infinity Feb 29, 2024
@wojtekmach wojtekmach merged commit 65590ff into main Feb 29, 2024
7 checks passed
@wojtekmach wojtekmach deleted the wm-add-docs-tarball-limits branch February 29, 2024 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Default tarball_max_size used by rebar3_hex too small
2 participants