Allow option overrides with global flow variables#78
Conversation
reddigari
requested review from
cesaragv,
diegostruk,
matthew-t-smith and
reelmatt
reelmatt
left a comment
reelmatt
left a comment
There was a problem hiding this comment.
A few missing parens/brackets are causing a compile error, but otherwise looks great!
The other comment re: where flow options appear can wait for later, but something to think about.
| disabled={props.disabled} | ||
| checked={value} | ||
| onChange={handleChange} /> | ||
|
|
There was a problem hiding this comment.
Missing some closing ) and } here. Causing a compile error when run.
| return (<></>) | ||
| } | ||
|
|
||
| const hideFlow = props.node.options.node_type === "flow_control" |
There was a problem hiding this comment.
KNIME allows for you to override flow node values with other flow nodes, so the first part here is not entirely correct. As-is though, I think this could cause an infinite loop on the server should a global flow node overwrite it's own value (it would keep looking up its own node). This shouldn't be a problem for local flow nodes (since they could never be provided as an option to themselves), so maybe change the check to props.node.is_global?
|
@reelmatt Great catch, both resolved by the last two commits. |
Now, raw filenames/inputs are stored in the Node (removing the construction of a path on Node update - in `node/views.py`). When a file is needed, the path is constructed at that time (either Node execution or file download).
|
@reelmatt your commit got it to execute properly, but the downloaded file was named |
|
We had something like this earlier, adding the filename in the response like: That should work now, but feel free to tweak that as you see fit. In general, I think that's a lot cleaner/simpler than trying to re-derive the filename from the |
4 participants
Any non-file parameters in the node configuration form have a checkbox indicating whether to override with a flow variable (if any exist). Checking the box disables the normal parameter input, and displays a dropdown to select the flow node to use as the override.
Currently no verification that the selected flow var has the same type as the option being overridden, but I think I'll handle that as I do local flow nodes tomorrow.
I've tested by overriding the
sepoption of ReadCsvNode to read a CSV where I swapped the commas for semicolons, and it works fine. There is a problem that @reelmatt is looking into where if you try to use a flow variable to override the WriteCsvNode output file, the server doesn't know how to construct the correct path.