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.

Sign up for GitHub

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

Jump to bottom

Feature/add umccrise rnasum and glues #525

Merged
merged 5 commits into from
Sep 2, 2024
Merged

Feature/add umccrise rnasum and glues #525

merged 5 commits into from
Sep 2, 2024

Conversation

alexiswl
Copy link
Member

@alexiswl alexiswl commented Sep 1, 2024 •
edited
Loading

Updates

  • Use portal run id for database uuid in icav2 services (since refId is no longer generated if it doesnt exist)
  • Use portal run id as idempotency key, allows for lambda retries if the workflow launch fails
  • Use upper case (screaming case) for statuses
  • Added linkedLibraries for WorkflowRunStateChange payloads
  • Added consistency for stacky rule names (will be useful for replay events, particularly if the FilterArns allows for the asterisk on suffixes)
  • Added project level events for when fastq list rows are available.
  • Added project name and owner to library event objects
  • Updated fastq list row event schema such that rgid is now the fastq list row id.
  • Added arriba output to the wts pipeline (required input for rnasum)

@alexiswl
Updated stacky with new metadata manager
8b7628d
@alexiswl
Initialise rnasum
48b3474
@alexiswl
Large stacky changes to fix linked libraries
492e85f 
Added linked libraries for each of the workflow inputs / outputs

Fixes to umccrise inputs (and outputs)

Refactor fastq list row ids, now uses rgid as the fastq list row id (which has the instrument run id and library id)

Added project level data functions
@alexiswl
Various stacky fixes for umccrise + rnasum
d034033 
* Use portal_run_id over ref id for database id (since refId isn't always propagated)
* Use idempotency key (set to portal run id) to prevent duplicate analyses
* Various fixes for linkedLibraries
* Various fixes for using new fastqListRow object syntax (and using rgid as the key)
* Add subjectId to wts tags (required by umccrise)
* Fixes to umccrise glue after testing
* Fixes to rnasum glue after testing
* Use new output_prefix syntax for dragen inputs
@alexiswl alexiswl self-assigned this Sep 1, 2024
@victorskl victorskl added the feature New feature label Sep 1, 2024
@victorskl victorskl linked an issue Sep 1, 2024 that may be closed by this pull request
Add umccrise and rnasum to glue stack #490
Closed
@victorskl
Copy link
Member

Review in tick, next

reisingerf
reisingerf approved these changes Sep 1, 2024
View reviewed changes
Copy link
Member

@reisingerf reisingerf left a comment

Choose a reason for hiding this comment

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

LGTM
(didn't go through all the details though)

Comment / question for upcoming releases:
There are a few constants that are "hard coded" or SSM parameter controlled, such as pipeline versions, ref files, ...
We could think about whether that info should come from the system itself, e.g. the workflow IDs could come from the workflow manager and/or the journey service, ref data is that a constant or dependent on the workflow / analysis use case (and could be annotated there instead)?

@alexiswl
Copy link
Member Author

alexiswl commented Sep 2, 2024

There are a few constants that are "hard coded" or SSM parameter controlled, such as pipeline versions, ref files, ...
We could think about whether that info should come from the system itself, e.g. the workflow IDs could come from the workflow manager and/or the journey service, ref data is that a constant or dependent on the workflow / analysis use case (and could be annotated there instead)?

Yes certainly, I think a lot of these hard-coded parameters would be better off in a terraform file or equivalent

victorskl
victorskl approved these changes Sep 2, 2024
View reviewed changes
Copy link
Member

@victorskl victorskl left a comment

Choose a reason for hiding this comment

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

LGTM

@victorskl victorskl mentioned this pull request Sep 2, 2024
Merged
@alexiswl
Copy link
Member Author

alexiswl commented Sep 2, 2024

Let's do it!

@alexiswl alexiswl merged commit cc0c911 into main Sep 2, 2024
5 checks passed
@alexiswl alexiswl deleted the feature/add-umccrise-rnasum-and-glues branch September 2, 2024 02:02
@alexiswl alexiswl mentioned this pull request Sep 2, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@reisingerf reisingerf reisingerf approved these changes

@victorskl victorskl victorskl approved these changes

Assignees

@alexiswl alexiswl

Labels
feature New feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add umccrise and rnasum to glue stack
3 participants
@alexiswl @victorskl @reisingerf