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.

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

TSPS-447 add file suffix to pipeline details response, update array_impuation version, remove admin update pipeline regex #207

Merged
merged 2 commits into from
Mar 6, 2025

Conversation

jsotobroad
Copy link
Collaborator

@jsotobroad jsotobroad commented Mar 5, 2025

add file_suffix value to pipeline details inputs section
removed regex that forced tool version to match pipeline version
update array_imputation version to 1

Description

Please replace this description with a concise description of this Pull Request.

Jira Ticket

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

Checklist (provide links to changes)

Screenshot 2025-03-05 at 4 35 27 PM

Screenshot 2025-03-05 at 4 35 50 PM

add file_suffix value to pipeline details inputs section
removed regex that forced tool version to match pipeline version
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.

Assuming the file suffix field doesn't show up in the response when it's null in the db, looks great!

Edit: I see the screenshots in the PR description now, thanks!

@@ -91,7 +91,8 @@ static ApiPipelineWithDetails pipelineWithDetailsToApi(Pipeline pipelineInfo) {
new ApiPipelineUserProvidedInputDefinition()
.name(input.getName())
.type(input.getType().toString())
.isRequired(input.isRequired()))
.isRequired(input.isRequired())
.fileSuffix(input.getFileSuffix()))
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think if this is null it just won't show up in the response json, is that right? Can you put an example response in the PR description that shows both file and non file types (ie the array imputation response)?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nvm I see the screenshots now, thanks!


// ensure that major version of toolVersion matches the value of the pipeline version.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sigh, sorry.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

😭

@@ -62,7 +62,7 @@ void getCorrectNumberOfPipelines() {
pipelinesRepository.save(
new Pipeline(
PipelinesEnum.ARRAY_IMPUTATION,
1,
2,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we make this the current test version + 1? Don't feel strongly, there aren't as many places to update as I feared

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ooo this feels nicer, yea lemme do that

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ended up creating a variable in that test class as it wasnt really used anywhere else to warrant putting it in teh test utils

@jsotobroad jsotobroad merged commit 0f6ba2b into main Mar 6, 2025
15 checks passed
@jsotobroad jsotobroad deleted the js_odds_and_ends branch March 6, 2025 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants