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

feat: collect extra shas for gitlab notif #698

Merged
merged 3 commits into from
Sep 16, 2024

Conversation

giovanni-guidini
Copy link
Contributor

depends on codecov/shared#354

These changes collect extra SHAs that we need to (potentially) notify for GitLab users
that use "merge request result" pipelines and the "merge train" feature.

These changes only collect the extra SHAs. The actual notification will happen later.

Copy link

This PR includes changes to shared. Please review them here: codecov/shared@42f83ec...f54ab4f

@codecov-notifications
Copy link

codecov-notifications bot commented Sep 10, 2024

Codecov Report

Attention: Patch coverage is 95.23810% with 2 lines in your changes missing coverage. Please review.

✅ All tests successful. No failed tests found.

Files Patch % Lines
tasks/notify.py 89.47% 2 Missing ⚠️

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #698      +/-   ##
==========================================
- Coverage   98.09%   98.09%   -0.01%     
==========================================
  Files         434      434              
  Lines       36607    36643      +36     
==========================================
+ Hits        35911    35945      +34     
- Misses        696      698       +2     
Flag Coverage Δ
integration 98.09% <95.23%> (-0.01%) ⬇️
latest-uploader-overall 98.09% <95.23%> (-0.01%) ⬇️
unit 98.09% <95.23%> (-0.01%) ⬇️

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

Components Coverage Δ
NonTestCode 96.00% <90.00%> (-0.01%) ⬇️
OutsideTasks 98.10% <100.00%> (-0.01%) ⬇️
Files Coverage Δ
services/comparison/__init__.py 97.11% <100.00%> (+0.01%) ⬆️
tasks/tests/unit/test_notify_task.py 100.00% <100.00%> (ø)
tasks/notify.py 96.33% <89.47%> (-0.52%) ⬇️

... and 4 files with indirect coverage changes

@codecov-qa
Copy link

codecov-qa bot commented Sep 10, 2024

Codecov Report

Attention: Patch coverage is 95.23810% with 2 lines in your changes missing coverage. Please review.

Project coverage is 98.09%. Comparing base (8e96f4d) to head (e1f4a6b).
Report is 2 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
tasks/notify.py 89.47% 2 Missing ⚠️

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #698      +/-   ##
==========================================
- Coverage   98.09%   98.09%   -0.01%     
==========================================
  Files         434      434              
  Lines       36607    36643      +36     
==========================================
+ Hits        35911    35945      +34     
- Misses        696      698       +2     
Flag Coverage Δ
integration 98.09% <95.23%> (-0.01%) ⬇️
latest-uploader-overall 98.09% <95.23%> (-0.01%) ⬇️
unit 98.09% <95.23%> (-0.01%) ⬇️

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

Components Coverage Δ
NonTestCode 96.00% <90.00%> (-0.01%) ⬇️
OutsideTasks 98.10% <100.00%> (-0.01%) ⬇️
Files with missing lines Coverage Δ
services/comparison/__init__.py 97.11% <100.00%> (+0.01%) ⬆️
tasks/tests/unit/test_notify_task.py 100.00% <100.00%> (ø)
tasks/notify.py 96.33% <89.47%> (-0.52%) ⬇️

... and 4 files with indirect coverage changes

Copy link

codecov-public-qa bot commented Sep 10, 2024

Codecov Report

Attention: Patch coverage is 95.23810% with 2 lines in your changes missing coverage. Please review.

Project coverage is 98.09%. Comparing base (8e96f4d) to head (e1f4a6b).
Report is 1 commits behind head on main.

✅ All tests successful. No failed tests found.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #698      +/-   ##
==========================================
- Coverage   98.09%   98.09%   -0.01%     
==========================================
  Files         434      434              
  Lines       36607    36643      +36     
==========================================
+ Hits        35911    35945      +34     
- Misses        696      698       +2     
Flag Coverage Δ
integration 98.09% <95.23%> (-0.01%) ⬇️
latest-uploader-overall 98.09% <95.23%> (-0.01%) ⬇️
unit 98.09% <95.23%> (-0.01%) ⬇️

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

Components Coverage Δ
NonTestCode 96.00% <90.00%> (-0.01%) ⬇️
OutsideTasks 98.10% <100.00%> (-0.01%) ⬇️
Files Coverage Δ
services/comparison/__init__.py 97.11% <100.00%> (+0.01%) ⬆️
tasks/tests/unit/test_notify_task.py 100.00% <100.00%> (ø)
tasks/notify.py 96.33% <89.47%> (-0.52%) ⬇️

... and 4 files with indirect coverage changes

Copy link

codecov bot commented Sep 10, 2024

Codecov Report

Attention: Patch coverage is 95.23810% with 2 lines in your changes missing coverage. Please review.

Project coverage is 98.14%. Comparing base (8e96f4d) to head (e1f4a6b).
Report is 2 commits behind head on main.

Changes have been made to critical files, which contain lines commonly executed in production. Learn more

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
tasks/notify.py Critical 89.47% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #698      +/-   ##
==========================================
- Coverage   98.14%   98.14%   -0.01%     
==========================================
  Files         475      475              
  Lines       37962    38278     +316     
==========================================
+ Hits        37257    37567     +310     
- Misses        705      711       +6     
Flag Coverage Δ
integration 98.09% <95.23%> (-0.01%) ⬇️
latest-uploader-overall 98.09% <95.23%> (-0.01%) ⬇️
unit 98.09% <95.23%> (-0.01%) ⬇️

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

Components Coverage Δ
NonTestCode 96.15% <90.00%> (+0.03%) ⬆️
OutsideTasks 98.10% <100.00%> (-0.01%) ⬇️
Files with missing lines Coverage Δ
services/comparison/__init__.py 97.13% <100.00%> (+0.01%) ⬆️
tasks/tests/unit/test_notify_task.py 100.00% <100.00%> (ø)
tasks/notify.py Critical 96.33% <89.47%> (-0.52%) ⬇️

... and 4 files with indirect coverage changes

This change has been scanned for critical changes. Learn more

@giovanni-guidini giovanni-guidini requested a review from a team September 10, 2024 18:20
@@ -529,6 +539,9 @@ def save_patch_totals(self, comparison: ComparisonProxy) -> None:
return
head_commit = comparison.head.commit
db_session = head_commit.get_db_session()
if db_session is None:
Copy link
Contributor

Choose a reason for hiding this comment

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

under which circumstances does this ever happen?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure. I don't think it should happen, but my type hinter tells me this the result of Commit.get_db_session() is Any | None.

Also we have issues WORKER-N5V and WORKER-NH1 - not in this task exactly - that show dbsession being None

depends on codecov/shared#354

These changes collect extra SHAs that we need to (potentially) notify for GitLab users
that use "merge request result" pipelines and the "merge train" feature.

These changes only _collect_ the extra SHAs. The actual notification will happen later.
Copy link

This PR includes changes to shared. Please review them here: codecov/shared@42f83ec...cdf263f

@giovanni-guidini giovanni-guidini added this pull request to the merge queue Sep 16, 2024
Merged via the queue into main with commit 26de475 Sep 16, 2024
24 of 27 checks passed
@giovanni-guidini giovanni-guidini deleted the gio/gitlab-extra-shas branch September 16, 2024 13:50
Copy link

sentry-io bot commented Sep 16, 2024

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ TorngitServer5xxCodeError: Github is having 5xx issues app.tasks.notify.Notify View Issue
  • ‼️ RemoteProtocolError: Server disconnected without sending a response. app.tasks.notify.Notify View Issue
  • ‼️ ConnectTimeout: HTTPSConnectionPool(host='api.github.com', port=443): Max retries exceeded with url: /app/install... app.tasks.notify.Notify View Issue

Did you find this useful? React with a 👍 or 👎

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.

2 participants