8000 Fix `joined=False` functionality by Swatinem · Pull Request #874 · codecov/worker · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
This repository was archived by the owner on May 5, 2025. It is now read-only.

Fix joined=False functionality #874

Merged
merged 1 commit into from
Nov 12, 2024
Merged

Fix joined=False functionality #874

merged 1 commit into from
Nov 12, 2024

Conversation

Swatinem
Copy link
Contributor

This functionality was temporarily broken due to parallel processing, and this restores that functionality.

Additionally, this also logs any time this flag is triggered to Sentry to assist in figuring out who is actually using this.

@Swatinem Swatinem requested a review from a team November 11, 2024 10:45
@Swatinem Swatinem self-assigned this Nov 11, 2024
@codecov Codecov
Copy link
codecov bot commented Nov 11, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 97.56098% with 1 line in your changes missing coverage. Please review.

Project coverage is 97.96%. Comparing base (95d1a40) to head (8a95d64).
Report is 7 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
services/report/tests/unit/test_process.py 94.11% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #874      +/-   ##
==========================================
+ Coverage   97.92%   97.96%   +0.03%     
==========================================
  Files         444      444              
  Lines       35436    35431       -5     
==========================================
+ Hits        34701    34710       +9     
+ Misses        735      721      -14
Flag Coverage Δ
integration 41.91% <39.02%> (+<0.01%) ⬆️
unit 90.75% <90.24%> (+0.03%) ⬆️

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

Components Coverage Δ
NonTestCode 95.93% <100.00%> (+0.01%) ⬆️
OutsideTasks 95.77% <88.57%> (+0.04%) ⬆️
Files with missing lines Coverage Δ
services/processing/merging.py 94.44% <100.00%> (+0.63%) ⬆️
services/report/__init__.py 96.96% <100.00%> (ø)
services/report/raw_upload_processor.py 98.92% <100.00%> (+1.83%) ⬆️
tasks/tests/unit/test_upload_finisher_task.py 100.00% <100.00%> (ø)
tasks/tests/unit/test_upload_processing_task.py 100.00% <100.00%> (ø)
services/report/tests/unit/test_process.py 94.22% <94.11%> (+4.86%) ⬆️

@codecov-notifications
Copy link
codecov-notifications bot commented Nov 11, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 97.56098% with 1 line in your changes missing coverage. Please review.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
services/report/tests/unit/test_process.py 94.11% 1 Missing ⚠️

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #874      +/-   ##
==========================================
+ Coverage   97.92%   97.96%   +0.03%     
==========================================
  Files         444      444              
  Lines       35436    35431       -5     
==========================================
+ Hits        34701    34710       +9     
+ Misses        735      721      -14
Flag Coverage Δ
integration 41.91% <39.02%> (+<0.01%) ⬆️
unit 90.75% <90.24%> (+0.03%) ⬆️

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

Components Coverage Δ
NonTestCode 95.93% <100.00%> (+0.01%) ⬆️
OutsideTasks 95.77% <88.57%> (+0.04%) ⬆️
Files with missing lines Coverage Δ
services/processing/merging.py 94.44% <100.00%> (+0.63%) ⬆️
services/report/__init__.py 96.96% <100.00%> (ø)
services/report/raw_upload_processor.py 98.92% <100.00%> (+1.83%) ⬆️
tasks/tests/unit/test_upload_finisher_task.py 100.00% <100.00%> (ø)
tasks/tests/unit/test_upload_processing_task.py 100.00% <100.00%> (ø)
services/report/tests/unit/test_process.py 94.22% <94.11%> (+4.86%) ⬆️

@codecov-qa
Copy link
codecov-qa bot commented Nov 11, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 97.56098% with 1 line in your changes missing coverage. Please review.

Project coverage is 97.96%. Comparing base (95d1a40) to head (8a95d64).
Report is 7 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
services/report/tests/unit/test_process.py 94.11% 1 Missing ⚠️

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #874      +/-   ##
==========================================
+ Coverage   97.92%   97.96%   +0.03%     
==========================================
  Files         444      444              
  Lines       35436    35431       -5     
==========================================
+ Hits        34701    34710       +9     
+ Misses        735      721      -14
Flag Coverage Δ
integration 41.91% <39.02%> (+<0.01%) ⬆️
unit 90.75% <90.24%> (+0.03%) ⬆️

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

Components Coverage Δ
NonTestCode 95.93% <100.00%> (+0.01%) ⬆️
OutsideTasks 95.77% <88.57%> (+0.04%) ⬆️
Files with missing lines Coverage Δ
services/processing/merging.py 94.44% <100.00%> (+0.63%) ⬆️
services/report/__init__.py 96.96% <100.00%> (ø)
services/report/raw_upload_processor.py 98.92% <100.00%> (+1.83%) ⬆️
tasks/tests/unit/test_upload_finisher_task.py 100.00% <100.00%> (ø)
tasks/tests/unit/test_upload_processing_task.py 100.00% <100.00%> (ø)
services/report/tests/unit/test_process.py 94.22% <94.11%> (+4.86%) ⬆️

@codecov-public-qa Codecov Public QA
Copy link
codecov-public-qa bot commented Nov 11, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 97.56098% with 1 line in your changes missing coverage. Please review.

Project coverage is 97.96%. Comparing base (95d1a40) to head (8a95d64).

✅ All tests successful. No failed tests found.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #874      +/-   ##
==========================================
+ Coverage   97.92%   97.96%   +0.03%     
==========================================
  Files         444      444              
  Lines       35436    35431       -5     
==========================================
+ Hits        34701    34710       +9     
+ Misses        735      721      -14
Flag Coverage Δ
integration 41.91% <39.02%> (+<0.01%) ⬆️
unit 90.75% <90.24%> (+0.03%) ⬆️

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

Components Coverage Δ
NonTestCode 95.93% <100.00%> (+0.01%) ⬆️
OutsideTasks 95.77% <88.57%> (+0.04%) ⬆️
Files Coverage Δ
services/processing/merging.py 94.44% <100.00%> (+0.63%) ⬆️
services/report/__init__.py 96.96% <100.00%> (ø)
services/report/raw_upload_processor.py 98.92% <100.00%> (+1.83%) ⬆️
tasks/tests/unit/test_upload_finisher_task.py 100.00% <100.00%> (ø)
tasks/tests/unit/test_upload_processing_task.py 100.00% <100.00%> (ø)
services/report/tests/unit/test_process.py 94.22% <94.11%> (+4.86%) ⬆️

@github-actions GitHub Actions
Copy link
github-actions bot commented Nov 11, 2024 •
edited
Loading

✅ All tests successful. No failed tests were found.

📣 Thoughts on this report? Let Codecov know! | Powered by Codecov

@Swatinem
Fix joined=False functionality
8a95d64 
This functionality was temporarily broken due to parallel processing, and this restores that functionality.

Additionally, this also logs any time this flag is triggered to Sentry to assist in figuring out who is actually using this.
@Swatinem Swatinem force-pushed the swatinem/fix-joined branch from 1030935 to 8a95d64 Compare November 11, 2024 10:58
giovanni-guidini
giovanni-guidini approved these changes Nov 12, 2024
View reviewed changes
Copy link
Contributor
@giovanni-guidini giovanni-guidini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't see anything in suspicious in the code, but no idea about the joined logic

@Swatinem
Copy link
Contributor Author

To be fair, I don’t think this is a widely used feature, and I also don’t think its particularly useful.

It pretty much means that followup uploads will fully override existing coverage values (for those files they do cover).

As the order of uploads is pretty much non-deterministic, I don’t think this works out well in practice.

@Swatinem Swatinem added this pull request to the merge queue Nov 12, 2024
Merged via the queue into main with commit 48e8163 Nov 12, 2024
24 of 27 checks passed
@Swatinem Swatinem deleted the swatinem/fix-joined branch November 12, 2024 09:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@giovanni-guidini giovanni-guidini giovanni-guidini approved these changes

Assignees

@Swatinem Swatinem

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Swatinem @giovanni-guidini
0