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

[23.0] Display DCE in job parameter component, allow rerunning with DCE input #15744

Merged
merged 6 commits into from
May 11, 2023

Conversation

mvdbeek
Copy link
Member

@mvdbeek mvdbeek commented Mar 8, 2023

Fixes all of #15729.

The big upshot is that element identifiers are correctly preserved, which would previously be lost, because the re-run would work over HDAs without the collection element context. But this is also much more correct and is the only way to re-run parts of jobs that consume entire subcollections.

How to test the changes?

(Select all options that apply)

  • I've included appropriate automated tests.
  • This is a refactoring of components with existing test coverage.
  • Instructions for manual testing are as follows:
    1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

  • I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

@mvdbeek mvdbeek changed the base branch from dev to release_23.0 March 8, 2023 20:03
@mvdbeek mvdbeek changed the title Display DCE in job parameter component [23.0] Display DCE in job parameter component Mar 8, 2023
@mvdbeek mvdbeek force-pushed the dce_display branch 4 times, most recently from 3265efb to af47306 Compare April 24, 2023 14:08
@mvdbeek mvdbeek force-pushed the dce_display branch 3 times, most recently from bb97680 to 51bd812 Compare May 8, 2023 16:22
mvdbeek added 3 commits May 9, 2023 11:04
If you navigate to a tool re-run url without loading a history first the
tool form would be rebuilt before the tool parameters from the re-run
were actually set, so you would effectively just get the default values
for the tool.
It'll always pull in the first dataset in the `other_values` dict, which
isn'r correct. Tool parameters at this point know how to deal with
collections, so this shouldn't be neceesary.
@mvdbeek mvdbeek changed the title [23.0] Display DCE in job parameter component [23.0] Display DCE in job parameter component, allow rerunning with DCE input May 10, 2023
@mvdbeek mvdbeek marked this pull request as ready for review May 10, 2023 13:56
@dannon dannon self-requested a review May 10, 2023 13:57
@github-actions github-actions bot added this to the 23.0 milestone May 10, 2023
@@ -47,6 +47,7 @@ export const SimpleProviderMixin = {
loading: this.loading,
item: this.item,
save: this.save,
result: this.item,
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor redundancy here but this allows it to plug into the generic item component without additional changes. We could resolve this later in dev.

Copy link
Contributor

@guerler guerler left a comment

Choose a reason for hiding this comment

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

Awesome work, for my test case it worked well and fixed the re-running with dce elements and also now displays the correct details on the job info page. This is a pretty important fix.

Copy link
Member

@dannon dannon left a comment

Choose a reason for hiding this comment

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

Thank you!

@dannon dannon merged commit 5ec64b9 into galaxyproject:release_23.0 May 11, 2023
@github-actions
Copy link

This PR was merged without a "kind/" label, please correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

4 participants