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

Allow users override maxInMemoryFileSize in multipartFileUpload #1087

Open
vkostyukov opened this issue Feb 21, 2019 · 1 comment
Open

Allow users override maxInMemoryFileSize in multipartFileUpload #1087

vkostyukov opened this issue Feb 21, 2019 · 1 comment

Comments

@vkostyukov
Copy link
Member

@vkostyukov vkostyukov commented Feb 21, 2019

It was asked on Gitter. Here is the argument we need to wire: https://github.com/twitter/finagle/blob/develop/finagle-base-http/src/main/scala/com/twitter/finagle/http/exp/MultipartDecoder.scala#L20

@vkostyukov vkostyukov added this to the Finch 1.0 milestone Feb 21, 2019
@vkostyukov vkostyukov removed this from the Finch 1.0 milestone Feb 27, 2019
@vkostyukov vkostyukov added this to the Finch 0.29 milestone Feb 27, 2019
@hderms
Copy link
Contributor

@hderms hderms commented Jun 6, 2019

@vkostyukov I started working on this one but I think some guidance on how you want the API to look would be useful. As of right now I see three possible high level ways to make this change:

  1. either add default values to API methods
    https://github.com/hderms/finch/blob/master/core/src/main/scala/io/finch/EndpointModule.scala#L367
    or adding additional method signatures (i.e. clone each api method dealing with multipart uploads and add a StorageUnit argument). Imo this is ugly but simple to understand. Also simple to deprecate particular variants of the API methods if we went with the additional method signatures approach.

  2. alternatively I could add a method like withMaxInMemoryUploadSize to https://github.com/hderms/finch/blob/master/core/src/main/scala/io/finch/endpoint/multipart.scala#L103
    which would make it a concern to the caller only if they specifically need to adjust the size. This seems undesirable based on the rest of the design of Finch

  3. Some kind of implicit parameter which you could provide your own instance of, which seems like a lot of machinery and doesn't seem correct from a design perspective

If you could push me in the right direction that would be appreciated. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants