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

Tool parameters now ignore 'refresh_on_change' attribute #4810

Closed
blankenberg opened this issue Oct 16, 2017 · 7 comments

Comments

Projects
None yet
4 participants
@blankenberg
Copy link
Member

commented Oct 16, 2017

This breaks tools using dynamic_options that rely on a previously set parameter's value. e.g. the Rsync with g2 data manager tool, that should reload select lists based upon dbkey change.

    <inputs>
        <param name="dbkey" type="genomebuild" label="dbkey to search for Reference Data" help="Specify ? to show all"/>

        <param name="data_table_names" type="select" display="checkboxes" multiple="True" optional="True" refresh_on_change="dbkey"
            label="Choose Desired Data Tables" dynamic_options="galaxy_code_get_available_data_tables( __trans__ )" />

        <param name="data_table_entries" type="select" display="checkboxes" multiple="true" optional="false" refresh_on_change="dbkey"
            label="Choose Desired Data Tables Entries" dynamic_options="galaxy_code_get_available_data_tables_entries( __trans__, dbkey, data_table_names )" />
    </inputs>
@guerler

This comment has been minimized.

Copy link
Contributor

commented Oct 16, 2017

I think refresh_on_change="True" should be set for the dbkey parameter, see: https://docs.galaxyproject.org/en/master/dev/schema.html#tool-code.

@blankenberg

This comment has been minimized.

Copy link
Member Author

commented Oct 16, 2017

What @guerler suggested does work, setting True on the parameter that you want to cause the refresh fixes things, and makes sense.

What is strange (concerning wrt backwards compatibility) is that this used to work with the refresh set to the target parameter, but perhaps that was a helpful bug...

@martenson

This comment has been minimized.

Copy link
Member

commented Oct 16, 2017

@blankenberg if it used to work that way we should probably just acknowledge the breaking change in the release notes I guess...

@blankenberg

This comment has been minimized.

Copy link
Member Author

commented Oct 17, 2017

@martenson indeed. I suppose that there are two possible scenarios:

  1. This attribute (was previously undocumented in this usage and it) had been working as intended (similar to data_meta --> ref of options) and behavior changed to a true/false.
  2. Some bug was allowing it to work when it should not have.

A quick grep didn't show any other tools that were using this attribute, so no clues there. The nice thing about having it reference a specific parameter is that it would be potentially fewer parameters that need to be updated due to a change. The nice thing about having the refresh being true for a parameter is that it simplifies things to always just refresh everything when that one parameter changes.

Normally I'd be all for trying to figure out what was going on before when this tool was working, but with limited time, I'll just accept that it should be True on the parameter that is changing.

@jmchilton

This comment has been minimized.

Copy link
Member

commented Oct 17, 2017

with limited time, I'll just accept that

Cleveland has changed you man, I don't even know who you are anymore.

@blankenberg

This comment has been minimized.

Copy link
Member Author

commented Oct 17, 2017

Cleveland has changed you man, I don't even know who you are anymore.

I know, right? I mean, I am totally for someone spending time to figure it out, and I do want to know, but alas...

@martenson

This comment has been minimized.

Copy link
Member

commented Oct 24, 2017

I added the functionality deprecation to the RN. I am not sure where to announce deprecation of @blankenberg though 😈

@martenson martenson closed this Oct 24, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.