Ignore dbkey option filter if dbkey is unset for dataset

[bernt-matthias]

Like in the filter for selects with dynamic options. Note actually in the select case we check if the referred dataset has the dbkey set while we do this here for the dataset itself. I guess this could be changed but I could not find a good way to do this.

Discovered while fixing https://github.com/galaxyproject/tools-iuc/pull/4023 for https://github.com/galaxyproject/galaxy/pull/12073

where this leads to a bug because filtered datasets are not matching: https://github.com/galaxyproject/galaxy/blob/5ea4d7e647d09dde957e824a083b1bd6acaa6f17/lib/galaxy/tools/parameters/dataset_matcher.py#L132

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

• [x] I agree to license these contributions under Galaxy's current license.
• [x] I agree to allow the Galaxy committers to license these and all my past contributions to the core galaxy codebase under the MIT license. If this condition is an issue, uncheck and just let us know why with an e-mail to [email protected].

[mvdbeek]

I think this makes a lot of sense, the dbkey mechanism is probably not widely used. But this is a bigger change.

I guess this could be changed but I could not find a good way to do this.

what do you want to change ?

[bernt-matthias]

what do you want to change ?

currenttly it is only checked if the dataset itself has a dbkey. Maybe it would be better to check if the dbkey is set for the referred dataset (which is what is happening for filters applied to selects with dynamic options).

But maybe it also makes sense to do both...?

[bernt-matthias]

Also I guess that the filter does not work without https://github.com/galaxyproject/galaxy/pull/12073 .. let me check if a test is already in place or if we need to add one ...

[bernt-matthias]

I fixed the patch .. at the moment there is no automatic way to test this since tool tests ignore this (https://github.com/galaxyproject/galaxy/pull/12073)

Anyway I added a tool tests over there (some of the tests will fail until this fix is included) .. if you want to see it in action I could temporarily cherry pick the commits of this PR to #12073.

But this is a bigger change.

OK. I guess a profile version could help :)?

[bernt-matthias]

at the moment there is no automatic way to test this

obviously I was wrong. failing tests guided me the way to add unit tests for the original and the profile version.

[bernt-matthias]

Not sure if the failing tests are related. I guess they are, but I tried to do what (I guess) the test does without problems.

[mvdbeek]

I am unsure about this, as much as I dislike the dbkey system we're arguably breaking the main use-case, filtering out datasets that aren't the right fit, here. I think that should probably an (advanced) option on the tool form if we implement this ?

[wm75]

Like in the filter for selects with dynamic options. Note actually in the select case we check if the referred dataset has the dbkey set while we do this here for the dataset itself.

Which is an important distinction I guess. It makes lots of sense to ignore dbkey if the referred to dataset doesn't declare one, but offering any dataset that itself doesn't declare one would effectively disable dbkeys filtering in most users' histories.

Regarding making this an advanced option as @mvdbeek suggests, isn't that "advanced" option in place already in the form of dragging the desired input over from the history?

[mvdbeek]

Regarding making this an advanced option as @mvdbeek suggests, isn't that "advanced" option in place already in the form of dragging the desired input over from the history?

That isn't being passed to the job submission, so we can't make this conditional.

[bernt-matthias]

Just to clarify this is about two data parameters A and B (I will add a functional tool test to make this clear). B has a filter that enforces to have the same dbkey as A.

• The previous behavior was that if A has no metadata set then there is no valid input for B (even if no metadata is set)
• The behavior after this change is that if A has no metadata set (I would read this as "A has no restriction on the metadata") then any input is valid

IMHO the later is what one actually wants.

Note that the change has no effect of tool testing. Since dataset filtering for tool tests is currently completely broken since nothing (except triggering implicit conversion) happens if they do not match:

https://github.com/galaxyproject/galaxy/blob/5f2b31b088b89bf5cc074cea5fb1b60ed9f68608/lib/galaxy/tools/parameters/basic.py#L2129

The functional test case also proves this. This is analogous to https://github.com/galaxyproject/galaxy/pull/12073

But that change is effective for the user since the to_dict function is correct https://github.com/galaxyproject/galaxy/blob/5f2b31b088b89bf5cc074cea5fb1b60ed9f68608/lib/galaxy/tools/parameters/basic.py#L2260

[bernt-matthias]

My biggest argument for a / the change is:

Currently tools using a dbkey filter for data params force users to use dbkeys even if

• a dbkey does not make sense for them
• they just don't want to use dbkeys

In this sense I would be completely fine with the suggestion of @wm75 to allow only datasets without a dbkey if the referred dataset has no dbkey (but then we also should change the metadata filter for dynamic selects .. but I'm not sure if @wm75's suggestion makes sense there).

Please force me to document this somewhere. To me it seems that the main problem here is that the aim and function of the dbkey mechanism is not documented somewhere (where I can find it .. please correct me if I am wrong).