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

Returned ETag for uploaded entities #244

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

adamsc64
Copy link

@adamsc64 adamsc64 commented Jun 26, 2022

Summary of Changes

  • If 'ETag' is present in service response, return it inside a metadata dictionary for both tasks.CompleteMultipartUploadTask and upload.PutObjectTask.
  • This change will allow integrations which rely on these call sites to access the ETag.

Background

Design decisions

  • I was unsure how to model the entity which would be returning the ETag information. I chose to follow the pattern I found inside upload.UploadPartTask, i.e. a pattern already in the codebase. This pattern returns the ETag inside an outer metadata dictionary.
  • Let me know if I should use another strategy, or if this general approach to the problem is problematic for some reason.

- If 'ETag' is present in service response, return it inside a metadata
  dictionary for both `tasks.CompleteMultipartUploadTask` and
  `upload.PutObjectTask`.
- This change will allow integrations which rely on these call sites to
  access the ETag.
@adamsc64
Copy link
Author

adamsc64 commented Jul 3, 2022

@kyleknap, I don't know what you think of these changes. Can I get a review? Also open to suggestions if you think someone else is better for the moment. Thanks so much.

@adamsc64
Copy link
Author

adamsc64 commented Aug 1, 2022

Hi @kyleknap. New contributor to boto and s3transfer here! Wondering what is the correct way to officially request a review of this by the s3transfer team. Thanks!

@cfxegbert
Copy link

It could be nice to return the versionId (optionally None) field also.

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

Successfully merging this pull request may close these issues.

2 participants