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

File ID Changes Across Hook Stages #1008

Closed
jimydavis opened this issue Sep 27, 2023 · 4 comments
Closed

File ID Changes Across Hook Stages #1008

jimydavis opened this issue Sep 27, 2023 · 4 comments
Labels

Comments

@jimydavis
Copy link
Contributor

Within the s3store.go file, there is this line:

info.ID = objectId + "+" + multipartId

which is somewhat conflicting with the promise of hookRes.ChangeFileInfo.ID .

In pre-create, the hook changes the File ID, but in post-create, after the s3 upload is completed, the ID is attached to the multipartID with a + sign.

Is there any promise implicit that the File ID will not change across the different stages or it is designed to not assume so?

Thank you.

@Acconut
Copy link
Member

Acconut commented Sep 28, 2023

That is correct. As mentioned in

// If ID is not empty, it will be passed to the data store, allowing
// hooks to influence the upload ID. Be aware that a data store is not required to
// respect a pre-defined upload ID and might overwrite or modify it. However,
// all data stores in the github.com/tus/tusd package do respect pre-defined IDs.
ID string

the exact details on how the ID from the pre-create hook is respected depends on each store. In the case of the S3Store, it has to append the S3 multipart upload ID because we cannot influence its value. In your hooks, you just have to split the ID by + and take the first part to get your custom ID.

@jimydavis
Copy link
Contributor Author

Thanks for the reply. This is just a suggestion but I was expecting the info file to have the multipart ID instead of being inside the URL itself. As the multipart ID is a S3 specific implementation detail, it would be abstracted away from the TusID. This is not a big deal and I am just going to be splitting with "+" for now.

@Acconut
Copy link
Member

Acconut commented Sep 28, 2023

We have thought about this in the past and there are pros and cons to such an approach. The benefit would be that the multipart ID is hidden from the user and the developers. A downside would be that we first have to look up the info file before we can fetch the status of the multipart upload. This can be done in parallel right now.

However, there are some ideas floating around to refactor the info file into its own provider, so that the upload information could be stored on Redis instead of a file on S3. Such an approach would also allow the developer to fully customize the upload ID.

@Acconut
Copy link
Member

Acconut commented Sep 18, 2024

With tusd v3 (release date unknown yet), the multipart ID will no longer be part of the upload URL for S3: #1167. Then the upload ID can be fully customized by the pre-create hook.

@Acconut Acconut closed this as completed Sep 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants