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

TSPS-401 add output expiration date to getPipelineRunResult response #187

Merged
merged 4 commits into from
Jan 16, 2025
Merged

TSPS-401 add output expiration date to getPipelineRunResult response #187

merged 4 commits into from
Jan 16, 2025

Conversation

jsotobroad
Copy link
Collaborator

Description

On successful runs, we want the service to return the expected expiration date of when the outputs will be deleted. This will help the user know by when they should download their data by.

Jira Ticket

https://broadworkbench.atlassian.net/browse/TSPS-401

example response from the getPipelineRunResult api on a successful run

{
  "jobReport": {
    "id": "3fa85f64-5717-4562-0012-2c963f66afa6",
    "description": "some description i think",
    "status": "SUCCEEDED",
    "statusCode": 200,
    "submitted": "2024-12-18T22:21:47.590524Z",
    "completed": "2024-12-19T16:14:56.446157Z",
    "resultURL": "http://localhost:8080/api/pipelineruns/v1/result/3fa85f64-5717-4562-0012-2c963f66afa6"
  },
  "pipelineRunReport": {
    "pipelineName": "array_imputation",
    "pipelineVersion": 0,
    "wdlMethodVersion": "ImputationBeagle_development_v0.0.2",
    "outputs": {
      "imputedMultiSampleVcfIndex": "https://storage.googleapis.com/fc-secure-972057b6-fce1-4ec9-be0c-0e5c97fdc3e8/submissions/final-outputs/7588fa5f-51d1-4ad3-8e43-777547b92026/ImputationBeagle/589fe094-c512-4dc3-af65-bee553167b56/call-GatherVcfs/min_quota.imputed.vcf.gz.tbi?X-Goog-Algorithm=GOOG4-RSA-SHA256&X-Goog-Credential=teaspoons-dev%40broad-dsde-dev.iam.gserviceaccount.com%2F20250114%2Fauto%2Fstorage%2Fgoog4_request&X-Goog-Date=20250114T205600Z&X-Goog-Expires=3600&X-Goog-SignedHeaders=host&X-Goog-Signature=63412a7a1eccd60a82f1920c1707902f261019a8e63797e752224ead1503d51c1a88efc516110ca4b0b49ec689d08be286a50d138f148d798b040a6dbea45290214fce3da6f79cb29c5ece26e4dd5bb6b7463abdb1198fd685d15999e0d2f393bf0509d0be9166ff60b6e490eaa142acb27f64e3829634d6bf9753922ac81d31e4a526fac4261c4a859aca7f62cb4a908cb836e22681326d24930284f486fbd82007177e548957f318ea41b55827014a69624f13ba12fa7a4b478213ddcabe8478369617df421e26df585922b393dcac876a968034a6ad615be8ea85a1831633ff94cdba310b651815e55f350173ee2aa6000e794e479dd8d6462e0eff153b94",
      "imputedMultiSampleVcf": "https://storage.googleapis.com/fc-secure-972057b6-fce1-4ec9-be0c-0e5c97fdc3e8/submissions/final-outputs/7588fa5f-51d1-4ad3-8e43-777547b92026/ImputationBeagle/589fe094-c512-4dc3-af65-bee553167b56/call-GatherVcfs/min_quota.imputed.vcf.gz?X-Goog-Algorithm=GOOG4-RSA-SHA256&X-Goog-Credential=teaspoons-dev%40broad-dsde-dev.iam.gserviceaccount.com%2F20250114%2Fauto%2Fstorage%2Fgoog4_request&X-Goog-Date=20250114T205600Z&X-Goog-Expires=3600&X-Goog-SignedHeaders=host&X-Goog-Signature=5bfc0418092c59727f3c80f68bae588f323ca8be5a7cafaf1c2fd8e3c3135d725aefb2f79035cc0b6661754918274965d39631bc221447b64dfae130a1dfe0bb879bd8f3c245116f2ca89e029be3ae2324171a11cc9da32116d9e33251338e9bfaf7254a61dd6f3796d52a95664092b38382c2091326b63654b325fb371bad7b9cea8653c73fc4efb6fd86315ee1fcffcbda831e35710f72eb4398a03c1c1a75975e4c28deee1bf997eafc8bf5c84e0f4feb77c011a30f5a3bebb58c3011d876aa757d8fa4a89ea520539977a9e419db7bf6a7273457785ffea52af2dd3c2f8c318e73f42522b7eca587cff3463cc60bcee01ef306b16ef928e167f2b5b52b53",
      "chunksInfo": "https://storage.googleapis.com/fc-secure-972057b6-fce1-4ec9-be0c-0e5c97fdc3e8/submissions/final-outputs/7588fa5f-51d1-4ad3-8e43-777547b92026/ImputationBeagle/589fe094-c512-4dc3-af65-bee553167b56/call-StoreChunksInfo/min_quota_chunk_info.tsv?X-Goog-Algorithm=GOOG4-RSA-SHA256&X-Goog-Credential=teaspoons-dev%40broad-dsde-dev.iam.gserviceaccount.com%2F20250114%2Fauto%2Fstorage%2Fgoog4_request&X-Goog-Date=20250114T205600Z&X-Goog-Expires=3600&X-Goog-SignedHeaders=host&X-Goog-Signature=2b32b52d119513beb255aee1fdf76d435aaa206e3397dce7a33d51c32546b03e138b76933ef47b3855c1026657d0d4fc0c7acd527639b575f5dc3dc7d49960e92b31789b0882d3d9310ec4394b06375e01722e4aaa2b880bee5f5cc1a327fc9ccaa3cae6bbb9ff001ff3a64683d3835f68bf1eaca2fde66f6688ff8e1038a6714e95a4f953fad3df919268e09e79849aff2aab4f41842a2d546f18b5dd59bc7039f7c151483ec782f46fd8d914b496afc8727b1b996d175646c4dbb09ae298e027d6cec9e7a805801a7bd94a800fc1f65cdb971dcfa8e8a164876d416a31f128a35badcf60deeb899e2cefcad1cd361b3ddb26703fbfd2d3d2df9ff5f2de74a8"
    },
    "outputExpirationDate": "2025-01-02T16:14:56.446157Z"
  }
}

example of a non successful run

{
  "jobReport": {
    "id": "3fa85f64-5717-4562-0011-2c963f66afa6",
    "status": "FAILED",
    "statusCode": 500,
    "submitted": "2024-12-12T23:17:35.778749Z",
    "completed": "2024-12-13T00:18:55.408490Z",
    "resultURL": "http://localhost:8080/api/pipelineruns/v1/result/3fa85f64-5717-4562-0011-2c963f66afa6"
  },
  "errorReport": {
    "message": "Not all runs succeeded for submission: 168aecb7-727c-40df-8259-28ff566a537a",
    "statusCode": 500,
    "causes": []
  },
  "pipelineRunReport": {
    "pipelineName": "array_imputation",
    "pipelineVersion": 0,
    "wdlMethodVersion": "ImputationBeagle_development_v0.0.2"
  }
}

mmorgantaylor
mmorgantaylor reviewed Jan 14, 2025
View reviewed changes
Copy link
Collaborator

@mmorgantaylor mmorgantaylor left a comment

Choose a reason for hiding this comment

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

i think this approach makes sense, a couple comments/suggestions

...c/main/java/bio/terra/pipelines/app/configuration/internal/PipelinesCommonConfiguration.java Outdated
import org.springframework.boot.context.properties.ConfigurationProperties;

@ConfigurationProperties(prefix = "pipelines.common")
public record PipelinesCommonConfiguration(Long storageBucketTtlDays) {}
Copy link
Collaborator

Choose a reason for hiding this comment

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

so we already have WdlPipelinesConfiguration, which contains stuff about quotas. Now that I think about it, quotas won't necessarily be specific to WdlPipelines - I like PipelinesCommonConfiguration better for that too. what do you think about merging the quota config stuff into this one?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

oh good point i totally forgot there was a WdlPipelinesConfiguration. let me see what each of them have and think about it a bit

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yea let me move it al to this new configuration and see what it looks liek then!

...c/main/java/bio/terra/pipelines/app/configuration/internal/PipelinesCommonConfiguration.java Outdated
import org.springframework.boot.context.properties.ConfigurationProperties;

@ConfigurationProperties(prefix = "pipelines.common")
public record PipelinesCommonConfiguration(Long storageBucketTtlDays) {}
Copy link
Collaborator

Choose a reason for hiding this comment

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

any reason for this to be a Long vs an Integer?

Copy link
Collaborator Author

@jsotobroad jsotobroad Jan 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

its onlly cuz the Instant.plus() function takes in a Long value and not an int

@jsotobroad jsotobroad changed the title add output expiration date to getPipelineRunResult response TSPS-401 add output expiration date to getPipelineRunResult response Jan 14, 2025
@jsotobroad jsotobroad marked this pull request as ready for review January 15, 2025 18:28
@jsotobroad jsotobroad mentioned this pull request Jan 15, 2025
Merged
mmorgantaylor
mmorgantaylor approved these changes Jan 16, 2025
View reviewed changes
Copy link
Collaborator

@mmorgantaylor mmorgantaylor left a comment

Choose a reason for hiding this comment

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

one variable renaming request but otherwise looks great!

...c/main/java/bio/terra/pipelines/app/configuration/internal/PipelinesCommonConfiguration.java Outdated
private Long quotaConsumedPollingIntervalSeconds;
private boolean quotaConsumedUseCallCaching;
private Long storageBucketTtlDays;
Copy link
Collaborator

Choose a reason for hiding this comment

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

minor, but could we name this something other than "storageBucket" since that's also the term we use for where we store the ref panel. maybe "userDataTtlDays" (I considered "outputTtlDays" but this applies to user inputs too)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yea much better, changing

@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@jsotobroad jsotobroad merged commit 1137821 into main Jan 16, 2025
15 checks passed
@jsotobroad jsotobroad deleted the js_TSPS-401 branch January 16, 2025 20:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mmorgantaylor mmorgantaylor mmorgantaylor approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jsotobroad @mmorgantaylor