nhost/hasura-storage

Support max_upload_file_size > 50mb default

fungilation opened this issue ยท 11 comments

Trying to set default bucket max_upload_file_size to 5gb per file hits this error in hasura console:

image

max_upload_file_size on storage.buckets should be a bigint

/cc @elitan @nunopato

Thanks for reporting this. Makes sense to fix this. I'd like to have a quick discussion on how to fix this because we can't simply change the type of the field as that'd change the API, potentially breaking user's code. Specially if we want to support >4GB files as that requires a custom scalar (64 bits numbers are not directly supported by the graphql spec)

My proposal is as follows:

Add new columns max_upload_file_size_unit and min_upload_file_size_unit of type strings. This fields could only be either b, k, m or g and basically would dictate whether the values in {min,max}_upload_file_size are defined as bytes, kilobytes, megabytes or gigabytes. The fields would default to b to keep the existing behavior.

Alternatively we could add new min/max columns where we allow representing file sizes as "50m", "4g" but I think that complicates the migration path as we need to somehow sync old and new columns while users migrate (which we could do with triggers I guess). It may also be a bit more brittle as users could introduce values that can't be parsed so we'd need to account for that.

Thoughts?

I like adding size unit. That should preserve backward compat, with default to bytes.

I'd suggest to KISS and add just a single column upload_file_size_unit for both min and max. I see little use case where someone would need min and max to be different units. As a matter of fact, one would want unit the same for both for consistency: either all bytes or all mb, etc.

Yeah, there are theoretical cases where that might not work (imagine wanting a very small minimum amount and a very large maximum amount) but they are probably unrealistic cases so I am leaning towards a single column like you suggested

This looks good to me.

stale commented

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Not stale.

stale commented

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Not stale still wanted

Is there any progress on this?

No progress yet as this hasn't been a priority. If this is a must for you don't hesitate to reach out to us via discord/email so we can discuss it. Thanks!