AJ-1602: list-jobs API should aggregate Import Service and cWDS #1308
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1308 +/- ##
===========================================
- Coverage 69.31% 69.19% -0.13%
===========================================
Files 97 98 +1
Lines 3406 3441 +35
Branches 369 377 +8
===========================================
+ Hits 2361 2381 +20
- Misses 1045 1060 +15 ☔ View full report in Codecov by Sentry. |
| val importServiceResponse = List( | ||
| ImportServiceListResponse("jobId1", "status1", "filetype1", None), | ||
| ImportServiceListResponse("jobId2", "status2", "filetype2", None) | ||
| ) |
There was a problem hiding this comment.
It weirds me out that these two list have identical jobIds. Not clear how/if that will impact the test
There was a problem hiding this comment.
copy/paste error, fixed!
| } | ||
|
|
||
| private def toImportServiceStatus(cwdsStatus: GenericJob.StatusEnum): String = { | ||
| STATUS_TRANSLATION.getOrElse(cwdsStatus, "blah") |
There was a problem hiding this comment.
Maybe this could throw an error if there's no translation?
There was a problem hiding this comment.
I added a code comment; I would prefer to not fail, but now this returns "Unknown" instead of the draft-for-early-review "blah"
| // get jobs from cWDS | ||
| cwdsJobs = cwdsDAO.listJobsV1(workspace.workspace.workspaceId, runningOnly)(userInfo) | ||
| } yield { | ||
| // merge Import Service and cWDS results |
There was a problem hiding this comment.
not for this PR, but maybe for the metrics theme: this might be a good spot to add some metric instrumentation to understand migration status:
- measure latency of each dispatch for comparison
- measure the # of jobs returned from each for comparison
AJ-1602
High-level changes:
CwdsDAOtrait, withHttp*andMock*implementationscwds.enabled(see https://github.com/broadinstitute/terra-helmfile/pull/5261):open questions:
for a future PR:
Have you read CONTRIBUTING.md lately? If not, do that first.
I, the developer opening this PR, do solemnly pinky swear that:
In all cases: