feat: create ta_finish_upload #1193
Conversation
🚨 Sentry detected 2 potential issues in your recent changes 🚨Lower risk findings
Did you find this useful? React with a 👍 or 👎 |
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## main #1193 +/- ##
==========================================
+ Coverage 97.70% 97.74% +0.03%
==========================================
Files 457 459 +2
Lines 37089 37294 +205
==========================================
+ Hits 36238 36453 +215
+ Misses 851 841 -10
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found.
📢 Thoughts on this report? Let us know! |
| log = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| def get_upload_ids(commitid: str) -> dict[int, ReportSession]: |
There was a problem hiding this comment.
maybe get "successful" upload ids? Or something to differentiate this from a generic get by id
|
|
||
|
|
||
| def get_upload_error(upload_ids: list[int]) -> ErrorPayload | None: | ||
| error = UploadError.objects.filter(report_session_id__in=upload_ids).first() |
There was a problem hiding this comment.
so if there's multiple errors, we'd just return the first one? And is this deterministic?
There was a problem hiding this comment.
so if there's multiple errors, we'd just return the first one
yes
And is this deterministic
good point
| celery_app.send_task( | ||
| cache_test_rollups_task_name, | ||
| kwargs={ | ||
| "repoid": repo.repoid, |
There was a problem hiding this comment.
is it too late to change this kwarg param to be repo_id to match the process_flakes_task?
There was a problem hiding this comment.
i think i'll do that as part of a separate change
| process_flakes_task_name, | ||
| kwargs={ | ||
| "repo_id": repo.repoid, | ||
| "commit_id": commit.commitid, |
There was a problem hiding this comment.
similar to the comment below, commitid and commit_id is confusing.
the first one is actually the commit_sha, and the X_id looks a lot like a primary key id, though it isn’t in this case.
| ) | ||
|
|
||
|
|
||
| def check_seat_activation(db_session: Session, pull: EnrichedPull) -> bool: |
There was a problem hiding this comment.
I wonder if such a function already exists someplace else?
There was a problem hiding this comment.
factored it out 👍
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
since all callers are using repo_id by this point we no longer have to accept repoid as an arg
this module is responsible for the new implementation of the TA finisher and it gets its test run and flake information from timescale and the new postgres flake table
joseph-sentry
force-pushed
the
joseph/ta-finisher
branch
from
e808b5b to
e6fffd8
Compare
this module is responsible for the new implementation of the TA finisher and it gets its test run and flake information from timescale and the new postgres flake table