optional data column is empty instead of none, when starting workflow in simplified workflow runform #12081

foellmelanie · 2021-05-31T16:39:56Z

Hi,

I have a tool with an optional=TRUE data column in a workflow. When I use the full workflow form this optional parameter is set to 'None' and the tool works However, when I start the workflow in the simplified workflow runform the parameter is empty and the tool crashes because it is expecting 'None' in case the parameter is not used.

Cheers,
Melanie

foellmelanie · 2021-05-31T16:40:59Z

this happened with Galaxy 21.01 on EU Server

mvdbeek · 2021-05-31T18:14:32Z

@foellmelanie did you connect an optional data input in the workflow editor ? If not we shouldn't actually show the simplified workflow form.

foellmelanie · 2021-05-31T18:34:57Z

No I didn't connect an optional data input.

bgruening · 2021-05-31T19:07:13Z

It seems that @kxk302 actually discovered the same? or a very similar bug.

You can see this in this history: https://usegalaxy.eu/u/bgruening/h/imported-cnnnewhistory

The workflow run failed, this time even with expanded-workflow-run form according to @kxk302 ... but a simple rerun of the failed tool returns green. You can see that the Choose a parameter name (with current value) in both screenshots are None and which should not happen.

The parameter in question is defined as optional as well (that references an optional input).

https://github.com/bgruening/galaxytools/blob/e2a5eade6d0e5ddf3a47630381a0ad90d80e8a04/tools/sklearn/main_macros.xml#L1449-L1458

mvdbeek · 2021-06-03T09:23:25Z

@foellmelanie which tool were you using and could you provide a minimal example ?

mvdbeek · 2021-06-03T09:24:35Z

You can see that the Choose a parameter name (with current value) in both screenshots are None and which should not happen.

And which one is correct here ? The empty value, right ?

bgruening · 2021-06-03T10:28:54Z

None I think - at least with None it turns green in both cases. I think @foellmelanie has also a str() cast https://github.com/galaxyproteomics/tools-galaxyp/blob/master/tools/cardinal/classification.xml#L118

mvdbeek · 2021-06-03T10:36:11Z

I think this is fixed in 21.05 / dev. https://gist.github.com/mvdbeek/a9bddc301f2cf507d5f48002754fa31b fails on usegalaxy.eu with parameter 'sp_name': requires a value, but no legal values defined but it works locally.

bgruening · 2021-06-03T10:42:36Z

We are not sure if we upgrade before GCC. Any clue what fixed that?

mvdbeek · 2021-06-03T10:44:02Z

You should really really update before GCC, please! This was a mess with last GCC having different version of Galaxy running. The release is stable now, no breaking bugs that we're aware of. It should go out early next week.

mvdbeek · 2021-06-03T11:12:38Z

Any clue what fixed that?

I don't know for sure, but I'd assume it's this one: #11265. Please take it as a sign that you should update ASAP, I'm sure there are more things like that. I will confirm and backport.

bgruening · 2021-06-03T11:20:02Z

I will talk to Gianmauro and see what he thinks.

mvdbeek · 2021-06-03T16:30:47Z

Alright, so I think this is kind of difficult to fix and happens because we cast the select in https://github.com/bgruening/galaxytools/blob/e2a5eade6d0e5ddf3a47630381a0ad90d80e8a04/tools/sklearn/main_macros.xml#L1451 to a text field in the workflow editor, since the editor can't access datasets. All values of a text field are cast to strings, so our null value becomes "", and that fails in https://github.com/galaxyproject/galaxy/blob/dev/lib/galaxy/tools/parameters/basic.py#L934, because the value is not a python None (which it should be in this case).

So we could take the shortcut and change if self.optional and self.tool.profile < 18.09 or value == "": to if self.optional and self.tool.profile < 18.09 or value == "": ... but that's not great, because we hardcode the shortcomings of the client form values on the backend (in the most ugly part of Galaxy). The advantage is that this will work for existing workflows, but the downside is that we'd need to keep this workaround forever, and it breaks the somewhat weird case of a user specifying "" (which we use surprisingly often in the iuc for option values). "" might correspond to an actual selected option value, not None, which means the same considerations as in galaxyproject/tools-iuc#1842 (comment) apply

Ideally we'd have a text field that can discriminate between "" and null for this case.

nsoranzo · 2021-06-03T17:03:03Z

Any chance this can be fixed in the workflow editor instead?

mvdbeek · 2021-06-03T18:53:46Z

Yeah, that's my suggestion. You can add a Boolean Set Value? that captures the difference between unset and set to ""

kxk302 · 2021-06-12T15:37:35Z

@mvdbeek I am still seeing the issue Bjorn reported. The first time I run this workflow, it fails. If I re-run it from the failed step onwards, it passes.

Here is the history: https://usegalaxy.eu/u/kaivan/h/gcc2021cnn

I get this message: Tool Error An error occurred with this dataset: parameter 'sp_name': requires a value, but no legal values defined

I reported the issue. Will paste the stack trace when I receive the email.

Can we meet briefly on Monday to discuss this? Thx.

kxk302 · 2021-06-12T15:38:57Z

Here is the stack trace:

Traceback (most recent call last):
  File "/opt/galaxy/server/lib/galaxy/jobs/runners/__init__.py", line 237, in prepare_job
    job_wrapper.prepare()
  File "/opt/galaxy/server/lib/galaxy/jobs/__init__.py", line 1126, in prepare
    tool_evaluator.set_compute_environment(compute_environment, get_special=get_special)
  File "/opt/galaxy/server/lib/galaxy/tools/evaluation.py", line 81, in set_compute_environment
    visit_input_values(self.tool.inputs, incoming, validate_inputs)
  File "/opt/galaxy/server/lib/galaxy/tools/parameters/__init__.py", line 170, in visit_input_values
    visit_input_values(input.cases[values['__current_case__']].inputs, values, callback, new_name_prefix, label_prefix, parent_prefix=name_prefix, **payload)
  File "/opt/galaxy/server/lib/galaxy/tools/parameters/__init__.py", line 174, in visit_input_values
    visit_input_values(input.inputs, values, callback, new_name_prefix, label_prefix, parent_prefix=name_prefix, **payload)
  File "/opt/galaxy/server/lib/galaxy/tools/parameters/__init__.py", line 162, in visit_input_values
    visit_input_values(input.inputs, d, callback, new_name_prefix, new_label_prefix, parent_prefix=new_name_prefix, **payload)
  File "/opt/galaxy/server/lib/galaxy/tools/parameters/__init__.py", line 176, in visit_input_values
    callback_helper(input, input_values, name_prefix, label_prefix, parent_prefix=parent_prefix, context=context)
  File "/opt/galaxy/server/lib/galaxy/tools/parameters/__init__.py", line 137, in callback_helper
    new_value = callback(**args)
  File "/opt/galaxy/server/lib/galaxy/tools/evaluation.py", line 79, in validate_inputs
    value = input.from_json(value, request_context, context)
  File "/opt/galaxy/server/lib/galaxy/tools/parameters/basic.py", line 934, in from_json
    raise ParameterValueError("requires a value, but no legal values defined", self.name, is_dynamic=self.is_dynamic)
galaxy.tools.parameters.basic.ParameterValueError: parameter 'sp_name': requires a value, but no legal values defined

bgruening · 2021-06-12T15:51:07Z

And this was now with 21.05.

mvdbeek · 2021-06-12T16:28:24Z

It's what I've explained in #12081 (comment). You can download the workflow and set the sp_name field to null instead of \"\", but that's all I can offer for now.

On the tool side I'd recommend not using optional parameters like that, maybe replace them with a conditional if they are truly optional.

kxk302 · 2021-06-12T23:48:18Z

FYI, I re-ran the training job and it succeeds. So, the problem now is in the workflow created prior to the fix. I searched the workflow file for 'sp_name' and here is what I found:

"sp_name": null, "sp_value": ""

So, sp_name is already null.

I could re-create the workflow and see if the saved file has anything different.

kxk302 · 2021-06-13T00:21:51Z

So, I re-created the workflow and the job completed successfully, and re-ran it in a new history and got the same error. Marius, I'll ping you on Monday if you have a few mins to discuss.

mvdbeek · 2021-06-13T14:32:45Z

That's a second bug we've been chasing for a while, good to know how to reproduce it.

kxk302 · 2021-06-13T17:45:09Z

So, I made the following change to the workflow file:

Before:
"hyperparams_swapping": {"infile_params": null, "param_set": [{"index": 0, "sp_name": null, "sp_value": ""}]}

After:
"hyperparams_swapping": {"infile_params": null, "param_set": []}

And workflow now run to completion successfully (Before this change, tried setting the sp_value to null, but it still failed).

If the `Set value for this optional select field ? `switch is set to no we serialize the value as `null` instead of `""`, which is especially important in optional select fields, where `""` may correspond to an option that is distinct from not setting a value. I have only enabled this for select parameters that are cast to text parameters, but I think this discrimination between set and unset would also be great for optional text parameters. There might be a better way to do it in the UI (maybe some icon that indicates that the value is set, that can be toggled with an backspace when the field is empty?), but I think this will do for the relatively rare case of optional select parameters that are cast to text fields. Fixes galaxyproject#12081

mvdbeek · 2021-06-14T15:43:51Z

and re-ran it in a new history and got the same error.

That's #9886

If the `tool_inputs` column of the `workflow_step` is loaded as MutableJSONType any "accidental" change of values in the dict marks the object as "dirty" in the sqlalchemy session it is attached to and will be persisted when the session is being flushed. Making the column type `JSONType` instead of `MutableJSONType` means that the object is not marked as dirty and will not be persisted in that case. The correct method to use to actually persist step inputs is WorkflowModule.save_to_step or setting step.tool_inputs directly. This fixes galaxyproject#9886 and galaxyproject#12081 (comment) The issue in galaxyproject#9886 is that when you submit a workflow the step state is converted to a tool state in https://github.com/mvdbeek/galaxy/blob/90dff9af5bec8bed1258620e5351d0350e284206/lib/galaxy/workflow/modules.py#L661: ``` def step_state_to_tool_state(self, state): state = safe_loads(state) if "format" in state: formats = state["format"] if formats: formats = ",".join(listify(formats)) state["format"] = formats state = json.dumps(state) return state ``` and safe_loads just returns the same object if the object that is passed in is not actually a JSON string. So `state["format"] = formats` manipulates the step state in place and that gets committed when flushing the new invocation.

mvdbeek added area/client area/workflows kind/bug labels May 31, 2021

mvdbeek mentioned this issue Jun 14, 2021

[21.05] Add switch for optional select in workflow editor #12138

Merged

5 tasks

mvdbeek mentioned this issue Jun 14, 2021

[21.05] Do not persist tool state when invoking workflow #12141

Merged

5 tasks

mvdbeek closed this as completed in 90ccfef Jun 15, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

optional data column is empty instead of none, when starting workflow in simplified workflow runform #12081

optional data column is empty instead of none, when starting workflow in simplified workflow runform #12081

foellmelanie commented May 31, 2021

foellmelanie commented May 31, 2021

mvdbeek commented May 31, 2021

foellmelanie commented May 31, 2021

bgruening commented May 31, 2021

mvdbeek commented Jun 3, 2021 •

edited

mvdbeek commented Jun 3, 2021 •

edited

bgruening commented Jun 3, 2021

mvdbeek commented Jun 3, 2021

bgruening commented Jun 3, 2021

mvdbeek commented Jun 3, 2021 •

edited

mvdbeek commented Jun 3, 2021

bgruening commented Jun 3, 2021

mvdbeek commented Jun 3, 2021 •

edited

nsoranzo commented Jun 3, 2021

mvdbeek commented Jun 3, 2021

kxk302 commented Jun 12, 2021

kxk302 commented Jun 12, 2021

bgruening commented Jun 12, 2021

mvdbeek commented Jun 12, 2021 •

edited

kxk302 commented Jun 12, 2021

kxk302 commented Jun 13, 2021

mvdbeek commented Jun 13, 2021

kxk302 commented Jun 13, 2021

mvdbeek commented Jun 14, 2021

optional data column is empty instead of none, when starting workflow in simplified workflow runform #12081

optional data column is empty instead of none, when starting workflow in simplified workflow runform #12081

Comments

foellmelanie commented May 31, 2021

foellmelanie commented May 31, 2021

mvdbeek commented May 31, 2021

foellmelanie commented May 31, 2021

bgruening commented May 31, 2021

mvdbeek commented Jun 3, 2021 • edited

mvdbeek commented Jun 3, 2021 • edited

bgruening commented Jun 3, 2021

mvdbeek commented Jun 3, 2021

bgruening commented Jun 3, 2021

mvdbeek commented Jun 3, 2021 • edited

mvdbeek commented Jun 3, 2021

bgruening commented Jun 3, 2021

mvdbeek commented Jun 3, 2021 • edited

nsoranzo commented Jun 3, 2021

mvdbeek commented Jun 3, 2021

kxk302 commented Jun 12, 2021

kxk302 commented Jun 12, 2021

bgruening commented Jun 12, 2021

mvdbeek commented Jun 12, 2021 • edited

kxk302 commented Jun 12, 2021

kxk302 commented Jun 13, 2021

mvdbeek commented Jun 13, 2021

kxk302 commented Jun 13, 2021

mvdbeek commented Jun 14, 2021

mvdbeek commented Jun 3, 2021 •

edited

mvdbeek commented Jun 3, 2021 •

edited

mvdbeek commented Jun 3, 2021 •

edited

mvdbeek commented Jun 3, 2021 •

edited

mvdbeek commented Jun 12, 2021 •

edited