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 callbacks fire for all meta-request types #344

Merged
merged 11 commits into from
Aug 25, 2023

Conversation

graebm
Copy link
Contributor

@graebm graebm commented Aug 23, 2023

Description of changes:

  • progress_callback is now invoked for all meta-request types.
    • Previously, it was only invoked for multi-part PUT and multi-part COPY. It wasn't invoked when PUT or COPY was done as a single-part operation, and wasn't invoked for any type of GET.
  • progress_callbacks now fire sequentially, and do not overlap with the meta-request's other callbacks.
    • Previously, progress_callbacks could fire on different threads and overlap with each other (and overlap the body_callback).
    • TODO: telemetry_callback can still overlap with other callbacks. We should give it the same treatment.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@codecov-commenter
Copy link

codecov-commenter commented Aug 23, 2023

Codecov Report

Merging #344 (6ea1c3f) into main (75788fb) will decrease coverage by 0.02%.
The diff coverage is 97.41%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #344      +/-   ##
==========================================
- Coverage   89.52%   89.51%   -0.02%     
==========================================
  Files          17       17              
  Lines        4897     4929      +32     
==========================================
+ Hits         4384     4412      +28     
- Misses        513      517       +4     
Files Changed Coverage Δ
source/s3_auto_ranged_get.c 97.88% <91.66%> (-0.67%) ⬇️
source/s3_meta_request.c 92.60% <96.66%> (-0.74%) ⬇️
source/s3_auto_ranged_put.c 92.25% <100.00%> (+0.06%) ⬆️
source/s3_client.c 87.98% <100.00%> (-0.05%) ⬇️
source/s3_copy_object.c 82.83% <100.00%> (+1.69%) ⬆️
source/s3_default_meta_request.c 94.59% <100.00%> (+0.34%) ⬆️
source/s3_request.c 95.23% <100.00%> (+0.04%) ⬆️
source/s3_util.c 98.66% <100.00%> (ø)

@graebm graebm marked this pull request as ready for review August 23, 2023 23:17
include/aws/s3/private/s3_meta_request_impl.h Outdated Show resolved Hide resolved
include/aws/s3/private/s3_meta_request_impl.h Outdated Show resolved Hide resolved
source/s3_auto_ranged_get.c Show resolved Hide resolved
source/s3_auto_ranged_get.c Outdated Show resolved Hide resolved
include/aws/s3/s3_client.h Outdated Show resolved Hide resolved
source/s3_default_meta_request.c Show resolved Hide resolved
@graebm graebm merged commit a0ee6a9 into main Aug 25, 2023
@graebm graebm deleted the improved-progress-callback branch August 25, 2023 22:13
graebm added a commit to awslabs/aws-crt-java that referenced this pull request Aug 26, 2023
Update to latest submodules. This gets us the improved S3 progress callbacks, which now fire for every meta-request type, and no longer overlap. See: awslabs/aws-c-s3#344

```
aws-c-auth         v0.7.2 -> v0.7.3
aws-c-cal          v0.6.1 -> v0.6.2
aws-c-common       v0.9.0 -> v0.9.1
aws-c-event-stream v0.3.1 -> v0.3.2
aws-c-http         v0.7.11 -> v0.7.12
aws-c-io           v0.13.31 -> v0.13.32
aws-c-mqtt         v0.9.3 -> v0.9.5
aws-c-s3           v0.3.14 -> v0.3.16
aws-lc             v1.13.0 -> v1.14.0
s2n                v1.3.48 -> v1.3.50
```
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.

5 participants