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.

Sign up for GitHub

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

Jump to bottom

Revert "feat: CheckpointLogger v2: cleaner usage, reliability counters, more UploadFlow metrics" #151

Merged
merged 1 commit into from
Oct 18, 2023
Merged

Revert "feat: CheckpointLogger v2: cleaner usage, reliability counters, more UploadFlow metrics" #151

merged 1 commit into from
Oct 18, 2023

Conversation

matt-codecov
Copy link
Contributor

there's a bug in this PR - because UploadFlow no longer inherits from str, it can't be serialized when we try to pass the checkpoint data between tasks. this wasn't covered in tests

i'm working on the fix but worker has not been deployed in a while and there is another outstanding issue we need to fix, so reverting this and will re-submit with the fix when that dust has settled

@codecov-qa
Copy link

codecov-qa bot commented Oct 18, 2023 •
edited
Loading

Codecov Report

Merging #151 (0142861) into main (2b9491d) will decrease coverage by 0.01%.
The diff coverage is 98.73%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #151      +/-   ##
==========================================
- Coverage   98.44%   98.43%   -0.01%     
==========================================
  Files         347      346       -1     
  Lines       27253    27051     -202     
==========================================
- Hits        26828    26628     -200     
+ Misses        425      423       -2
Flag Coverage Δ
integration 98.43% <98.73%> (-0.01%) ⬇️
latest-uploader-overall 98.43% <98.73%> (-0.01%) ⬇️
unit 98.43% <98.73%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
NonTestCode 96.98% <98.57%> (-0.02%) ⬇️
OutsideTasks 98.23% <98.48%> (-0.01%) ⬇️
Files Coverage Δ
helpers/tests/unit/test_checkpoint_logger.py 100.00% <100.00%> (ø)
tasks/notify.py 98.78% <100.00%> (-0.05%) ⬇️
tasks/tests/unit/test_notify_task.py 100.00% <100.00%> (ø)
tasks/tests/unit/test_upload_finisher_task.py 100.00% <100.00%> (ø)
tasks/tests/unit/test_upload_task.py 99.44% <100.00%> (-0.01%) ⬇️
tasks/upload.py 98.26% <100.00%> (-0.01%) ⬇️
tasks/upload_finisher.py 95.32% <100.00%> (-0.13%) ⬇️
helpers/checkpoint_logger.py 98.33% <98.33%> (ø)

@codecov-public-qa
Copy link

codecov-public-qa bot commented Oct 18, 2023 •
edited
Loading

Codecov Report

Merging #151 (0142861) into main (2b9491d) will decrease coverage by 0.01%.
The diff coverage is 98.73%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #151      +/-   ##
==========================================
- Coverage   98.44%   98.43%   -0.01%     
==========================================
  Files         347      346       -1     
  Lines       27253    27051     -202     
==========================================
- Hits        26828    26628     -200     
+ Misses        425      423       -2
Flag Coverage Δ
integration 98.43% <98.73%> (-0.01%) ⬇️
latest-uploader-overall 98.43% <98.73%> (-0.01%) ⬇️
unit 98.43% <98.73%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
NonTestCode 96.98% <98.57%> (-0.02%) ⬇️
OutsideTasks 98.23% <98.48%> (-0.01%) ⬇️
Files Coverage Δ
helpers/tests/unit/test_checkpoint_logger.py 100.00% <100.00%> (ø)
tasks/notify.py 98.78% <100.00%> (-0.05%) ⬇️
tasks/tests/unit/test_notify_task.py 100.00% <100.00%> (ø)
tasks/tests/unit/test_upload_finisher_task.py 100.00% <100.00%> (ø)
tasks/tests/unit/test_upload_task.py 99.44% <100.00%> (-0.01%) ⬇️
tasks/upload.py 98.26% <100.00%> (-0.01%) ⬇️
tasks/upload_finisher.py 95.32% <100.00%> (-0.13%) ⬇️
helpers/checkpoint_logger.py 98.33% <98.33%> (ø)

@codecov
Copy link

codecov bot commented Oct 18, 2023

Codecov Report

Merging #151 (0142861) into main (2b9491d) will increase coverage by 0.06%.
The diff coverage is 98.73%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #151      +/-   ##
==========================================
+ Coverage   98.39%   98.46%   +0.06%     
==========================================
  Files         373      373              
  Lines       27749    27647     -102     
==========================================
- Hits        27305    27222      -83     
+ Misses        444      425      -19
Flag Coverage Δ
integration 98.43% <98.73%> (-0.01%) ⬇️
latest-uploader-overall 98.43% <98.73%> (-0.01%) ⬇️
unit 98.43% <98.73%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
NonTestCode 97.06% <98.57%> (+0.17%) ⬆️
OutsideTasks 98.23% <98.48%> (-0.01%) ⬇️
Files Coverage Δ
helpers/tests/unit/test_checkpoint_logger.py 100.00% <100.00%> (ø)
tasks/notify.py 98.78% <100.00%> (-0.05%) ⬇️
tasks/tests/unit/test_notify_task.py 100.00% <100.00%> (ø)
tasks/tests/unit/test_upload_finisher_task.py 100.00% <100.00%> (ø)
tasks/tests/unit/test_upload_task.py 99.44% <100.00%> (-0.01%) ⬇️
tasks/upload.py 98.26% <100.00%> (-0.01%) ⬇️
tasks/upload_finisher.py 95.37% <100.00%> (-0.13%) ⬇️
helpers/checkpoint_logger.py 98.33% <98.33%> (+31.66%) ⬆️
Related Entrypoints
run/app.tasks.notify.Notify
run/app.tasks.upload.UploadFinisher

@matt-codecov matt-codecov merged commit c2e6f61 into main Oct 18, 2023
18 of 26 checks passed
@matt-codecov matt-codecov deleted the revert-100-matt/checkpoint-logger-v2 branch October 18, 2023 21:23
matt-codecov added a commit that referenced this pull request Oct 18, 2023
@matt-codecov
Revert "Revert "feat: CheckpointLogger v2: cleaner usage, reliability…
503eb97 
… counters, more UploadFlow metrics" (#151)"

This reverts commit c2e6f61.
@matt-codecov matt-codecov mentioned this pull request Oct 18, 2023
Merged
matt-codecov added a commit that referenced this pull request Oct 20, 2023
@matt-codecov
(redo) feat: CheckpointLogger v2: cleaner usage, reliability counters…
7e263ca 
…, more UploadFlow metrics (#152)

* Revert "Revert "feat: CheckpointLogger v2: cleaner usage, reliability counters, more UploadFlow metrics" (#151)"

This reverts commit c2e6f61.

* fix bug with serializing between tasks and add test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@trent-codecov trent-codecov trent-codecov approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@matt-codecov @trent-codecov