Gh app backfill task extra #413
Conversation
| new_repo_service_ids=new_repo_service_ids, | ||
| ), | ||
| ) | ||
| gh_app_installation.repository_service_ids = new_repo_service_ids |
There was a problem hiding this comment.
Won't this overwrite what might be already there? Or are you sure at this point that the repository_service_ids is an empty list (when it shouldn't be)?
There was a problem hiding this comment.
Yeah this won't overwrite the things we have, see here for example:
worker/tasks/backfill_existing_gh_app_installations.py
Lines 61 to 62 in 163e7c9
worker/tasks/backfill_owners_without_gh_app_installations.py
Lines 83 to 84 in 163e7c9
There was a problem hiding this comment.
What I meant is, for example, if some installation G has repository_service_ids={1200}. Then this logic would overwrite the current selection {1200}.
But it doesn't really matter cause you are listing all the repos the installation has access to, which would include 1200.
There was a problem hiding this comment.
Yeah, I think this was part of the initial decision here, to also overwrite anything existing in case those were stagnant. And as you said, if it's "all", then that would contemplate anything that was already in
|
Worth noting there's a chance this could still error out due to memory, in which case I'd further add subtasks to each of these, but wanted to try this first |
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #413 +/- ##
==========================================
+ Coverage 97.44% 97.48% +0.03%
==========================================
Files 395 398 +3
Lines 33422 33379 -43
==========================================
- Hits 32568 32539 -29
+ Misses 854 840 -14
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #413 +/- ##
=======================================
Coverage 97.44% 97.44%
=======================================
Files 395 398 +3
Lines 33430 33431 +1
=======================================
+ Hits 32577 32578 +1
Misses 853 853
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found @@ Coverage Diff @@
## main #413 +/- ##
=======================================
Coverage 97.44% 97.44%
=======================================
Files 395 398 +3
Lines 33430 33431 +1
=======================================
+ Hits 32577 32578 +1
Misses 853 853
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #413 +/- ##
=======================================
Coverage 97.47% 97.47%
=======================================
Files 426 429 +3
Lines 34121 34122 +1
=======================================
+ Hits 33259 33260 +1
Misses 862 862
Flags with carried forward coverage won't be shown. Click here to find out more.
This change has been scanned for critical changes. Learn more |
| name=GITHUB_APP_INSTALLATION_DEFAULT_NAME, | ||
| ) | ||
| db_session.add(gh_app_installation) | ||
| db_session.commit() |
There was a problem hiding this comment.
I think you should wait to commit this change only after the repository selection is complete. Otherwise if there are issues with the repo selection we'd have a github installation with bad repo selection
There was a problem hiding this comment.
Makes sense. If I get rid of this line, both subsequent methods commit there so that takes care of this
…r into gh-app-backfill-task-extra
Last time this was ran the tasks ran out of memory. I'm wanting to do 2 things to help with this:
Logic remains the same, mostly moving files and tests arounds, renaming files/tasks, etc
Legal Boilerplate
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. In 2022 this entity acquired Codecov and as result Sentry is going to need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.