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

Resumable Uploads: Resource-specific upload limits #2747

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

Conversation

Acconut
Copy link
Member

@Acconut Acconut commented Mar 4, 2024

As discussed in #2741, this mechanism allows the server to send an Upload-Limit header in informational and final responses. The header indicates a maximum upload size that the server accepts. If the upload exceeds the size, the server may abort it depending on its preference. A client should not continue upload if it knows that its upload won't fit into that size.

Closes #2741.

@LPardue
Copy link
Contributor

LPardue commented Mar 4, 2024

Can the upload limit change during am upload? I don't think we should allow that

@guoye-zhang
Copy link
Contributor

Do we want Upload-Limit to be a dictionary structured field so it can support additional information in the future? E.g. Upload-Limit: bytes=100000000, concurrency=4

@Acconut
Copy link
Member Author

Acconut commented Mar 4, 2024

Can the upload limit change during am upload? I don't think we should allow that

Good point, I agree that should not be allowed.

@Acconut
Copy link
Member Author

Acconut commented Mar 4, 2024

Do we want Upload-Limit to be a dictionary structured field so it can support additional information in the future? E.g. Upload-Limit: bytes=100000000, concurrency=4

I don't see a reason against this. We could also use this to indicate a limit on the lifetime of an upload resource or the minimum and maximum sizes of individual PATCH requests. Since we get dictionaries for free from structured fields, we can make it a dictionary easily.

@Acconut
Copy link
Member Author

Acconut commented Mar 4, 2024

Added the changes. Let me know what you think

@Acconut Acconut changed the title Resumble Uploads: Resource-specific upload limits Resumable Uploads: Resource-specific upload limits Mar 4, 2024
@LPardue
Copy link
Contributor

LPardue commented Mar 4, 2024

Making it a dictionary is going to need more text than that I'm afraid. You're going to need an IANA registry etc . But most importantly we then need to define a framework for limits and what the expectations are for setting them. Immediately I can see from Guoye's example we are conflating limits on the upload resource with limits on the server more generally, and that sort of generic capability is an interop concern

We're also treading near to the rate limit headers in httpapi. The uses cases are distinct but there was a lot of friction about how to express these things over there so we need to be wary.

I'm not against the idea of Dictionary but this needs more WG discussion before landing

@Acconut
Copy link
Member Author

Acconut commented Mar 4, 2024

That makes sense. Maybe we are better of with individual headers to represent the individual limits. I see that individual headers are also more in line with how messages in HTTP are laid as this allows reuse etc.

On the top of my head, I can think of the following limits applying to an upload resource:

  • total size
  • minimum and maximum size of an upload creation or append request (e.g. limited by proxies or buffers)
  • lifetime of the resource
  • concurrent upload request (if that's specified in another document in the future)

For some of these there already seem to be existing headers. For example, lifetime could be expressed using the Sunset header.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Indicating general and resource-specific upload limits
3 participants