hansva opened a new pull request, #8101:
URL: https://github.com/apache/hop/pull/8101
Fixes #8084.
### The bug
A pipeline that declares its own named parameter with a default could have
that parameter
silently overwritten by an unrelated variable of the same name that happened
to exist in the
calling workflow's scope — even with *Pass parameter values to sub pipeline*
unchecked and no
entry for it on the Parameters tab. Two independently written pipelines that
both use a common
name (`my-param`, `input_file`, `date`) could contaminate each other.
### Root cause
`NamedParameters.activateParameters` gained "prefer an already-set variable
over the parameter's
default" in #7517, narrowed to non-empty defaults in #7521. Neither change
was about parameter
semantics: both were patching the single-JVM integration-test runner, where
a nested `main-*.hwf`
declaring `HOSTNAME` with default `localhost` stopped picking up the project
environment's
`HOSTNAME`. The workaround went into core, so it changed parameter
precedence for every pipeline
and workflow in the product.
Two further band-aids had grown on top of it: `PipelineExecutor` cleared
"sticky" values before
activation, and `ActionWorkflow` carried a comment pointing at the core
hack. Both are removed here.
The original integration-test problem is fixed properly rather than worked
around — see
*Behaviour changes* below. No changes were needed in
`integration-tests/scripts/`.
### One contract, six executors
Everything that starts a child pipeline or workflow now resolves parameters
through one shared
implementation, `org.apache.hop.core.parameters.SubExecutionParameters`:
1. A value on the caller's **Parameters** tab wins, whatever the checkbox
says.
2. Otherwise, if *Pass parent values to matching parameters* is on, the
caller's value of a
parameter or variable with that name.
3. Otherwise the child's own default.
A row that names a parameter the child does not declare is declared on the
child and set. A row
that configures nothing — no value, no stream field — is treated as if it
were not there, so the
option and the child's default decide. A row whose stream field is empty on
this row stays
authoritative and falls back to the child's default, which is what stops a
value from a previous
executor iteration sticking.
`TransformWithMappingMeta.activateParams` is kept and delegates, so plugins
calling it keep working.
| Executor | Before | Now |
|---|---|---|
| Pipeline action | ignored its own checkbox for unconfigured rows | shared
implementation |
| Workflow action | passed only formal parent *parameters* | shared
implementation |
| Pipeline Executor | shared logic plus a sticky-value workaround | shared
implementation, workaround deleted |
| Workflow Executor | **checkbox was persisted and drawn but never read at
runtime** | shared implementation, checkbox now works |
| Simple / Multi Mapping | shared logic | shared implementation |
| Metadata Injection | no options at all, always inherited everything |
Parameters tab + checkbox added |
| Repeat | `// subParams.activateParameters(); TODO` and a dead inheritance
loop | shared implementation, checkbox added |
Neither control has ever governed variable inheritance — the engine
factories call
`initializeFrom(parent)` unconditionally either way — so the six differently
worded labels were
replaced by one shared `System.Parameters.PassParentValues.Label`:
**"Pass parent values to matching parameters"**, with a tooltip stating that
variables are
inherited regardless. The six obsolete keys are removed from all locale
files so stale
translations cannot resurface.
`PipelineExecutorMeta.setDefault()` now sets the option on for newly created
transforms, matching
every other executor and its own documentation.
### Behaviour changes
Measured against a rebuilt 2.18-semantics client, not against the 2.19 bug.
**vs 2.19.0** — the fix itself. A declared parameter with a non-empty
default no longer adopts an
unrelated same-named variable when the option is off. The option is on by
default everywhere, so
most existing solutions are unaffected.
**vs 2.18** — three deliberate changes, all in the direction of the option
meaning something:
1. **Sub-workflows honour it.** A sub-workflow previously received only the
caller's formal
*parameters*; it now also receives a same-named *variable*, exactly as
sub-pipelines always
have. IT `main-0004`'s `TEST4_PARAMETER3` assertion is updated
accordingly — this is the one
assertion changed on purpose, and it is why #7521 had to revert half of
#7517.
2. **The Workflow Executor checkbox works.** Existing files carry
`inherit_all_vars=Y`, so their
children will now be seeded.
3. **The Pipeline action honours its own checkbox.** In 2.18 an unconfigured
grid row passed the
caller's variable whether the box was ticked or not. Untick it now and
the child keeps its
default — which is exactly what #8084 asks for.
(1) and (2) mean more values reach sub-workflows; (3) means fewer, and only
when someone has
deliberately unticked the box.
### Documentation
New reference page **Passing parameters to a child pipeline or workflow**
(`variables/parameter-passing.adoc`), linked from `variables.adoc`,
`fields-parameters-variables.adoc`, `nav.adoc` and all seven executor pages.
It states the rules,
what counts as "supplies a value", the full outcome table, and the cases
that surprise people.
Two pre-existing documentation errors are corrected:
- `fields-parameters-variables.adoc` advised giving a child *"a non-empty
default so an already-set
variable of the same name is kept"*. That sentence was written a month
after #7521 and described
the regression, not the contract.
- The Pipeline action page claimed you can *"NULL them by leaving the value
empty"*. An empty value
has always meant "nothing passed", so the child's default applies.
The executor pages no longer claim the checkbox controls variable
inheritance.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
