add needs_webhook_secret_backfill to upload task #521
Conversation
🔍 Existing Issues For ReviewYour pull request is modifying functions with the following pre-existing issues: 📄 File: tasks/upload.py
Did you find this useful? React with a 👍 or 👎 |
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #521 +/- ##
=======================================
Coverage 97.47% 97.48%
=======================================
Files 418 418
Lines 34900 34973 +73
=======================================
+ Hits 34020 34093 +73
Misses 880 880
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #521 +/- ##
=======================================
Coverage 97.47% 97.48%
=======================================
Files 418 418
Lines 34900 34973 +73
=======================================
+ Hits 34020 34093 +73
Misses 880 880
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found @@ Coverage Diff @@
## main #521 +/- ##
=======================================
Coverage 97.47% 97.48%
=======================================
Files 418 418
Lines 34900 34973 +73
=======================================
+ Hits 34020 34093 +73
Misses 880 880
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Changes have been made to critical files, which contain lines commonly executed in production. Learn more ✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #521 +/- ##
=======================================
Coverage 97.50% 97.50%
=======================================
Files 449 449
Lines 35623 35696 +73
=======================================
+ Hits 34733 34806 +73
Misses 890 890
Flags with carried forward coverage won't be shown. Click here to find out more.
|
tasks/upload.py
Outdated
| action="EDIT", | ||
| ), | ||
| ) | ||
|
|
||
| if webhook_secret is not None: |
There was a problem hiding this comment.
Couldn't this conditional/line be inside the if repository_service.service in ["gitlab", "gitlab_enterprise"]: check? So like
if repository_service.service in ["gitlab", "gitlab_enterprise"]:
# we use per-repo webhook secrets in this case
webhook_secret = repository.webhook_secret or str(uuid.uuid4())
repository.webhook_secret = webhook_secretAnd we then get rid of the this if statement
There was a problem hiding this comment.
I like it! updating
tasks/upload.py
Outdated
| repo_data["repo"]["hookid"] = hookid | ||
| return True | ||
|
|
||
| return True if should_post_webhook else False |
There was a problem hiding this comment.
With the other suggestion above, I think it'd be a bit more readable to put return True in the should_post_webhook conditional, and false in the else in 993, wdyt?
There was a problem hiding this comment.
agree! updating
set up the webhook_secret in passing if it is empty - starts the fix for https://github.com/codecov/internal-issues/issues/366