8000 Do not update `Commit.notified` by Swatinem · Pull Request #860 · 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.

Do not update Commit.notified #860

Merged
merged 3 commits into from
Nov 8, 2024
Merged

Do not update Commit.notified #860

merged 3 commits into from
Nov 8, 2024

Conversation

Swatinem
Copy link
Contributor
@Swatinem Swatinem commented Nov 7, 2024 •
edited
Loading

This field is constantly being updated/touched in both UploadFinisher and Notify.

But I couldn’t find any piece of code that actually reads this field across the whole codebase.

Moves and debounces the update to Repository.updatestamp, fixes https://github.com/codecov/internal-issues/issues/1043

Stops setting Commit.pullid in UploadFinisher.

This already happens as part of {PreProcess,}Upload, so it is not needed to set this field once again.

This also cleans up the code quite a bit.

Part of codecov/engineering-team#2824

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

Codecov Report

Attention: Patch coverage is 95.31250% with 3 lines in your changes missing coverage. Please review.

Project coverage is 97.96%. Comparing base (9d6bdc7) to head (eb9a68f).
Report is 3 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
tasks/upload_finisher.py 91.66% 3 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #860      +/-   ##
==========================================
- Coverage   97.97%   97.96%   -0.02%     
==========================================
  Files         442      442              
  Lines       35672    35641      -31     
==========================================
- Hits        34949    34915      -34     
- Misses        723      726       +3
Flag Coverage Δ
integration 97.96% <95.31%> (-0.02%) ⬇️
unit 97.96% <95.31%> (-0.02%) ⬇️

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

Components Coverage Δ
NonTestCode 95.99% <91.89%> (-0.03%) ⬇️
OutsideTasks 97.98% <ø> (ø)
Files with missing lines Coverage Δ
tasks/notify.py 96.12% <ø> (-0.03%) ⬇️
tasks/tests/unit/test_notify_task.py 100.00% <ø> (ø)
tasks/tests/unit/test_upload_finisher_task.py 100.00% <100.00%> (ø)
tasks/tests/unit/test_upload_processing_task.py 100.00% <100.00%> (ø)
tasks/tests/utils.py 100.00% <ø> (ø)
tasks/upload_processor.py 92.00% <100.00%> (-4.88%) ⬇️
tasks/upload_finisher.py 95.90% <91.66%> (-0.99%) ⬇️

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

Codecov Report

Attention: Patch coverage is 95.31250% with 3 lines in your changes missing coverage. Please review.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
tasks/upload_finisher.py 91.66% 3 Missing ⚠️

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #860      +/-   ##
==========================================
- Coverage   97.97%   97.96%   -0.02%     
==========================================
  Files         442      442              
  Lines       35672    35641      -31     
==========================================
- Hits        34949    34915      -34     
- Misses        723      726       +3
Flag Coverage Δ
integration 97.96% <95.31%> (-0.02%) ⬇️
unit 97.96% <95.31%> (-0.02%) ⬇️

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

Components Coverage Δ
NonTestCode 95.99% <91.89%> (-0.03%) ⬇️
OutsideTasks 97.98% <ø> (ø)
Files with missing lines Coverage Δ
tasks/notify.py 96.12% <ø> (-0.03%) ⬇️
tasks/tests/unit/test_notify_task.py 100.00% <ø> (ø)
tasks/tests/unit/test_upload_finisher_task.py 100.00% <100.00%> (ø)
tasks/tests/unit/test_upload_processing_task.py 100.00% <100.00%> (ø)
tasks/tests/utils.py 100.00% <ø> (ø)
tasks/upload_processor.py 92.00% <100.00%> (-4.88%) ⬇️
tasks/upload_finisher.py 95.90% <91.66%> (-0.99%) ⬇️

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

Codecov Report

Attention: Patch coverage is 95.31250% with 3 lines in your changes missing coverage. Please review.

Project coverage is 97.96%. Comparing base (9d6bdc7) to head (eb9a68f).
Report is 3 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
tasks/upload_finisher.py 91.66% 3 Missing ⚠️

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #860      +/-   ##
==========================================
- Coverage   97.97%   97.96%   -0.02%     
==========================================
  Files         442      442              
  Lines       35672    35641      -31     
==========================================
- Hits        34949    34915      -34     
- Misses        723      726       +3
Flag Coverage Δ
integration 97.96% <95.31%> (-0.02%) ⬇️
unit 97.96% <95.31%> (-0.02%) ⬇️

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

Components Coverage Δ
NonTestCode 95.99% <91.89%> (-0.03%) ⬇️
OutsideTasks 97.98% <ø> (ø)
Files with missing lines Coverage Δ
tasks/notify.py 96.12% <ø> (-0.03%) ⬇️
tasks/tests/unit/test_notify_task.py 100.00% <ø> (ø)
tasks/tests/unit/test_upload_finisher_task.py 100.00% <100.00%> (ø)
tasks/tests/unit/test_upload_processing_task.py 100.00% <100.00%> (ø)
tasks/tests/utils.py 100.00% <ø> (ø)
tasks/upload_processor.py 92.00% <100.00%> (-4.88%) ⬇️
tasks/upload_finisher.py 95.90% <91.66%> (-0.99%) ⬇️

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

Codecov Report

Attention: Patch coverage is 95.31250% with 3 lines in your changes missing coverage. Please review.

Project coverage is 97.96%. Comparing base (9d6bdc7) to head (eb9a68f).

✅ All tests successful. No failed tests found.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #860      +/-   ##
==========================================
- Coverage   97.97%   97.96%   -0.02%     
==========================================
  Files         442      442              
  Lines       35672    35641      -31     
==========================================
- Hits        34949    34915      -34     
- Misses        723      726       +3
Flag Coverage Δ
integration 97.96% <95.31%> (-0.02%) ⬇️
unit 97.96% <95.31%> (-0.02%) ⬇️

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

Components Coverage Δ
NonTestCode 95.99% <91.89%> (-0.03%) ⬇️
OutsideTasks 97.98% <ø> (ø)
Files Coverage Δ
tasks/notify.py 96.12% <ø> (-0.03%) ⬇️
tasks/tests/unit/test_notify_task.py 100.00% <ø> (ø)
tasks/tests/unit/test_upload_finisher_task.py 100.00% <100.00%> (ø)
tasks/tests/unit/test_upload_processing_task.py 100.00% <100.00%> (ø)
tasks/tests/utils.py 100.00% <ø> (ø)
tasks/upload_processor.py 92.00% <100.00%> (-4.88%) ⬇️
tasks/upload_finisher.py 95.90% <91.66%> (-0.99%) ⬇️

@Swatinem Swatinem force-pushed the swatinem/no-commit-notified branch 2 times, most recently from aebfd73 to 3402d59 Compare November 7, 2024 10:20
@Swatinem
Do not update Commit.notified
48b1376 
This field is constantly being updated/touched in both `UploadFinisher` and `Notify`.

But I couldn’t find any piece of code that actually reads this field across the whole codebase.
@Swatinem
move and debounce updates to Repository.updatestamp
54a10ce
@Swatinem Swatinem force-pushed the swatinem/no-commit-notified branch from 3402d59 to 134feb9 Compare November 7, 2024 10:30
@Swatinem
Stop setting Commit.pullid in UploadFinisher
eb9a68f 
This already happens as part of `{PreProcess,}Upload`, so it is not needed to set this field once again.
@Swatinem Swatinem force-pushed the swatinem/no-commit-notified branch from 134feb9 to eb9a68f Compare November 7, 2024 10:44
@Swatinem Swatinem added this pull request to the merge queue Nov 8, 2024
Merged via the queue into main with commit 8b1750a Nov 8, 2024
24 of 27 checks passed
@Swatinem Swatinem deleted the swatinem/no-commit-notified branch November 8, 2024 08:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@adrian-codecov adrian-codecov adrian-codecov 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 @adrian-codecov
0