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 future result in S3Transfer's download and upload #3316

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

Conversation

adamsc64
Copy link

Summary of changes

  • Adds a return statement to S3Transfer's methods upload_file() and download_file().
  • This gives them the same return behavior as copy(), upload_fileobj(), and download_fileobj() inside s3/inject.py. These functions return the value for their future objects.
  • Returning the value will allow integrations to access metadata returned by the service, such as the "ETag" for uploaded entities for example.

Rationale

  • There is currently no way to access metadata about S3 service operations when using upload_file() and upload_fileobj() on a bucket object. For example, before the changes in this PR, in this case:
    s3 = session.resource("s3")
    bucket = s3.Bucket("redacted")
    fp = io.BytesIO("test".encode("utf-8"))
    resp = bucket.upload_fileobj(fp, "test")
    
    the value for resp was None. This is in contrast to put_object(), which does return upload metadata.
  • One example of a good use case to access metadata is accessing the upload ETag. This is currently an open issue as documented here: S3 upload_file() and upload_fileobj() do not return the object's ETag. #2861.

Caveats

  • ETag information won't be available until another PR is merged in s3transfer, namely this PR: Returned ETag for uploaded entities s3transfer#244. However, these two PRs can be independently merged and are not necessarily related. One might want to merge this PR even indepenently for the sake of ensuring consistent return behavior with copy(), upload_fileobj(), and download_fileobj() inside inject.py.

- Adds a `return` statement to S3Transfer's methods `upload_file()` and
  `download_file()`.
- This gives them the same return behavior as `copy()`, `upload_fileobj()`,
  and `download_fileobj()` inside `s3/inject.py`. These functions return the
  value for their future objects.
- Returning the value will allow integrations to access metadata
  returned by the service, such as the "ETag" for uploaded entities for
  example.
@adamsc64
Copy link
Author

adamsc64 commented Jul 3, 2022

@jamesls Looking back in the history, I think this was your code originally, but I'm not sure. Can I get a review of this? Or feel free to let me know if you think someone else would be a better option.

@adamsc64
Copy link
Author

adamsc64 commented Aug 1, 2022

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

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.

None yet

1 participant