[TSPS-766] Add support for cloud-based file inputs#5483
Conversation
| "orchestrationUrlRoot": "https://firecloud-orchestration.dsde-dev.broadinstitute.org", | ||
| "rawlsUrlRoot": "https://rawls.dsde-dev.broadinstitute.org", | ||
| "teaspoonsUrlRoot": "https://teaspoons.dsde-dev.broadinstitute.org", | ||
| "teaspoonsUrlRoot": "http://localhost:8080", |
There was a problem hiding this comment.
minor directory re-org based on input type previously the structure was like:
.../inputs/PipelineBooleanInput.tsx
.../inputs/PipelineBooleanInput.test.tsx
.../inputs/PipelineStringInput.tsx
.../inputs/PipelineStringInput.test.tsx
...
now it's like:
.../inputs/boolean/PipelineBooleanInput.tsx
.../inputs/boolean/PipelineBooleanInput.test.tsx
.../inputs/string/PipelineStringInput.tsx
.../inputs/string/PipelineStringInput.test.tsx
...
Mainly useful for the FILE type because there are more file-related input components now for local/cloud
| return; | ||
| } | ||
|
|
||
| if (!path.startsWith('gs://')) { |
There was a problem hiding this comment.
do we want something similar to the FILE_NAME_VALIDATION_REGEX check we do for local files, or is that not necessary here? like how does this deal with spaces in the cloud path?
There was a problem hiding this comment.
good callout - i can make the validation a bit stronger
| fileInputUploadUrls = result.fileInputUploadUrls; | ||
| setPreparedJobId(preparedJobId); | ||
| } catch (error) { | ||
| handlePipelineSubmissionError(error, 'Failed to prepare pipeline run'); |
There was a problem hiding this comment.
access issues will be returned with /prepare response; I think handlePipelineSubmissionError will show that error message to the user, right?
There was a problem hiding this comment.
that error actually wasn't being bubbled up to the user because of an issue in handlePipelineSubmissionError, but i've fixed it. thanks for calling that out
|
| }) => { | ||
| const [cloudPath, setCloudPath] = useState(''); | ||
| const { isRequired, fileSuffix } = input; | ||
| const GCS_PATH_VALIDATION_REGEX = /^gs:\/\/[a-z0-9._-]+\/.+/; |
salonishah11
left a comment
salonishah11
left a comment
There was a problem hiding this comment.
Sonarcloud is reporting a minor unexpected negation issue but apart from that this LGTM
|
unless anyone disagrees, i prefer the way the code is written over what Sonar is suggesting for the negated condition warning |
4 participants
Jira Ticket: https://broadworkbench.atlassian.net/browse/TSPS-766
Summary of changes:
What
Adds support for gcs hosted file inputs.
Why
Testing strategy