-
Notifications
You must be signed in to change notification settings - Fork 101
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 optional Content-Location response header #159
base: main
Are you sure you want to change the base?
Conversation
I have had this requested for tusdotnet several times so glad you added it to the spec! Some thoughts from a generic lib point of view:
I like that you linked to the RFC 👍 |
ebf5701
to
32ab6ba
Compare
@smatsson thank you, I have changed the wording based on your feedback. I have now specified the methods to send a But you are definitely right regarding the word "permanent", that was not a good choice. From the protocol perspective, we should not be concerned about the future, we are just saying "this is the new location, deal with it".
This touches a fundamental question, and I think the right answer is: you can't know in the general case. The server might continue to serve the content under the upload url and I do not think the protocol should make the |
I agree with you here, the previous wording was better. I'm also not a fan of referring to
Hmm, after taking a step back, and not to diss you proposal here, but should this really be added to the spec? It is unknown when to expect the header and it's unknown what to do with the header once received. The only way to know is to have a contract with the specific server. Since there is nothing in the spec prohibiting the server from adding whatever header they feel like to any response, this seems to fit the bill better as an app specific thing? |
32ab6ba
to
b3e3416
Compare
... to inform the Client about the permanent location of completed uploads. This is motivated by the following use cases: * TUS servers may not implement storage themselves, such that the `Location` returned for a creation (POST) request be only transitional. * TUS servers may decide to change the storage location once the upload is complete, for example to * implement content-based naming (where the file name is some hash over the content) * name content based on type * or select paths by size The `Content-Location` header is already defined by the HTTP RFCs, so, stricly speaking, its use should be implicitly allowed and well defined. Yet, for TUS, it makes sense to allow use of this response header for additional HTTP verbs (e.g. `PATCH`) and it might help implementation interoperability to point it out.
b3e3416
to
f84236f
Compare
Re @smatsson OK, I have reverted to basically ebf5701, but avoiding to talk about permanent location. Yes, I agree, this is merely a pointer to the appropriate place in the HTTP RFCs, and it is true that any server could, with good reason, expect a client to handle the response correctly. In reality, however, implementors hardly ever read RFCs in their entirety and #155 is proof that it even took me quite a long time to come up with a reasonable suggestion. On top, we
|
I might be missing something but I don't really follow why this goes in the spec? It seems very domain specific to me. The server is free to include any header at any time given the current protocol (obviously except the ones that are reserved in the protocol). The headers reserved in the protocol are all well defined when it comes to when to expect them and what to do with them when they are received. In this proposal it seems that it's unknown for the client when the header can be received and it's unknown what to do with the header once received. Is it the URL where the client can download content? Is it the URL to use when polling for e.g. video encoding progress? Is the client supposed to continue all tus operations on the new location? To answer any of these questions the client will need to check the client/server agreement (or application-specific/domain-specific protocol or whatever one would call it). I'm not saying that I don't find the functionality useful (e.g. for your hash based content storage) but I'm not entirely sure that it's a general purpose feature that should be in the spec, given the above issues on how to interpret the header. In your use case in #155, how would the clients use this header and for what? |
re @smatsson
The client MAY receive the header and IF it does, it should refer to the resource via the URL given in the
I understand this was a rhetorical question, but the technical answer would be: As
Again, as the
No. If I failed to phrase the proposal well enough, I need to improve. But this definitely should be clear from the protocol text.
see also #159 (comment) Please do also keep in mind that the semantics of the
Say the Upload-URL (to More specifically to my use case, |
Hello! I'd like to hop into this discussion and note that I'd like to extend this a bit further and allow the server to allow with arbitrary metadata (See #164) once an upload is complete. I think that this would be more flexible than what this is proposing, and I'd be OK with "well" known values for common stuff like Content-Location, but in my case we're returning things like exif data and image dimensions. There can also be enough metadata that it could be a pretty large header, and I know some proxies have restrictions on header lengths that may be interesting to design around. |
A brief request: @nigoroll, it would be nice if you could avoid force-pushing commits to your PRs as it makes it impossible for others to see the history of changes. For example, you were discussing whether the specific HTTP methods should be included in the specification but since there is only one commit in this PR, I cannot see what exactly you talked about. We squash PRs in the end anyways, so the entire PR will become one commit anyways. Hope that's understandable :) Regarding the topic itself: As @rockwotj mentions, it may be helpful to make a more generic post-processing extension. Right now, this PR mostly talks about providing the new content location to the client, which is helpful if the server moves resources to other storage. However, I think that this is a very specific use case and since we have had many requests about dealing with post-processing in the past, it might be a good opportunity to attack now. My idea would be that a server could tell the client an URL under which additional information about the post-processing status is available. Maybe we could reuse the Link header for that (https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Link). The tus clients can then implement features to automatically fetch this status and provide it to the application. Since moving uploads to another storage is a type of post-processing, this approach could also be used to tell the client the new location. However, the mechanism is also flexible enough that it allows for other use cases, such as @rockwotj mentions. Does this make sense? |
@Acconut this sounds good to me 👍 |
@nigoroll Thanks for the explanation! That clears it up a lot. To be honest I still don't think this should be part of the tus protocol, mainly because it is already defined in the RFC and as you said it should already be supported by the client and server. I think that @Acconut's proposal is a better fit to allow more options on post processing than just where to download the file content. |
re @Acconut force-pushing is a habit from other projects which works because gh keeps and logs previous commit hashes for PRs. But I will try to remember that this is not how this project handles things. FTR, up to now the versions of this PR are:
The link (based on the link html element) is intended to provide just that, links. So it seems to me like a sensible choice for a post processing status URL, but I do not see how it would help with metadata, which is a property of the uploaded resource and not one of a resource where additional status information can be queried. (#164 seems to be a bit special in that @rockwotj said he had so much metadata that it would not fit in a header, so for that case I actually do agree that a link to another resource describing the upload resource could be an option). With respect to So, to summarize: put links in the |
I am open for adding a note about the Content-Location header to the specification and I will start a discussion about a separate post-processing extension in a new GitHub issue. However, I would change the wording of this PR slightly. @nigoroll, do you mind if I add a commit? |
@Acconut sorry for not responding earlier, please feel free |
... to inform the Client about the permanent location of completed uploads.
Continues #155
This is motivated by the following use cases:
TUS servers may not implement storage themselves, such that the
Location
returned for a creation (POST) request be only transitional.TUS servers may decide to change the storage location once the upload is complete, for example to
The
Content-Location
header is already defined by the HTTP RFCs, so, stricly speaking, its use should be implicitly allowed and well defined. Yet, for TUS, it makes sense to allow use of this response header for additional HTTP verbs (e.g.PATCH
) and it might help implementation interoperability to point it out.