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

Progress callback not called when adding a custom Checksum Algorithm to an upload_file request #3949

Closed
moretromain opened this issue Nov 27, 2023 · 2 comments
Assignees
Labels
duplicate This issue is a duplicate.

Comments

@moretromain
Copy link

Trying to revive #3782 as a new issue...here are my findings, with a potential fix, although I'm not completely sure about its side-effects...

I'm stubbling upon this issue as well, and after a little bit of debugging, I think I found the source of the issue, here: https://github.com/boto/botocore/blob/develop/botocore/httpchecksum.py#L332

When using a ChecksumAlgorithm, the request body (that is initially a ReadFileChunk) is wrapped into an AwsChunkedWrapper, which doesn't have a couple of methods, such as signal_transferring()/signal_not_transferring().
As a result, the hook on "request-created.s3.UploadPart" (which is this: https://github.com/boto/s3transfer/blob/develop/s3transfer/utils.py#L56) is not calling anything, thus not enabling the calls to the progress callbacks.

I'll try to understand all this a bit further, I'm new to boto3 so I'm not sure what would be the proper way to fix this...maybe simply decorate signal_transferring()/signal_not_transferring() in the AwsChunkedWrapper to redirect to the wrapped object ? i'll give it a quick try, see what happens...

Edit:

@tim-finnigan, adding this:

    def signal_transferring(self):
        self._raw.signal_transferring()

    def signal_not_transferring(self):
        self._raw.signal_not_transferring()

...to AwsChunkedWrapper did the trick and the progress callback is now called when using a ChecksumAlgorithm.

But I have no idea if this is a good fix or not...

Originally posted by @moretromain in #3782 (comment)

@tim-finnigan tim-finnigan self-assigned this Dec 15, 2023
@tim-finnigan
Copy link
Contributor

Hi @moretromain thanks for reaching out. I'm going to follow up in the original issue and close this as a duplicate.

@tim-finnigan tim-finnigan added the duplicate This issue is a duplicate. label Dec 15, 2023
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

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

No branches or pull requests

2 participants