Skip to content

fix(frontend): hold the Python UDF parameter table's Name and Type cells locked when the panel enables its form group - #8848

Merged
aicam merged 1 commit into
apache:mainfrom
yangzhang75:fix/udf-parameters-lock-under-panel-enable
Nov 5, 2026
Merged

aicam merged 1 commit into
apache:mainfrom
yangzhang75:fix/udf-parameters-lock-under-panel-enable

Conversation

@yangzhang75

@yangzhang75 yangzhang75 commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

Closes #8847. The Python UDF operator's "UI parameters" table (#5043) infers each parameter's name and type from the self.UiParameter(...) declarations in the operator's code, so the widget disables those two columns' controls as it builds each row: a name typed into the table does not reach the code, and the parameter's value then never reaches it either (checked on main: the table's name changes, the code's does not).

The lock does not survive on the operator property panel. setInteractivity(true) enables the whole form group -- one task after every render, and again whenever the workflow becomes modifiable, for instance when a run ends -- and formly mirrors a control's state back into props.disabled, so the one-time disable is simply undone. Which state a given panel opening shows depends on which of the two ran last: measured on main, the cells were editable on 4 of 5 openings, and always after a run. The Form View, which never enables a group wholesale, has always shown them locked, so the two views disagreed as well.

The widget now holds its own lock. On each cell it locks it registers Angular's disabled-change hook of the control (registerOnDisabledChange, the same hook formly's mirror and the input's value accessor use): when a host enables the control, the hook disables it again in the microtask after that enable, and announces the status change as the enable did. Both details matter and were found on a running instance:

  • deferred to the microtask, because the input's value accessor is one more hook on the same control, registered after this one when the cell renders; disabling from inside the enable's own round of hooks left the accessor having been told "enabled" last, so the box was editable over a locked control;
  • announced rather than silent (disable(), not disable({emitEvent: false})), because ng-zorro's nz-input paints its disabled attribute from the control's status changes; a silent disable left it painting the box editable again at the next change detection.

A cell the widget configures editable -- the Value column, or any cell once reconfigured -- is released from the hold, so whether it is editable stays the host's call: a read-only panel still locks everything, and a host that disables the whole field still reaches the Value cell. The frame is untouched: two other approaches were tried first and dropped, re-disabling from the frame after enable() (formly's mirror has already set props.disabled to false by then) and a formly expression on props.disabled (formly re-evaluates expressions on model changes only, and registers them at populate, so enable() never triggers one).

Any related issues, documentation, discussions?

Closes #8847. Split out of #8846 (the same table under the Form View's renames and hides), which is based on main and does not touch this; the two are independent and can land in either order. The table's fixed 250px columns clip Name and Type at the panel's default 260px width, which is why the cells have to be brought into view to see any of this; that is a separate layout bug, filed separately.

How was this PR tested?

Unit tests (vitest). The widget's spec: a locked cell's control is held locked when a host enables it, in the microtask after the enable and announcing the status change, while the Value cell is left to the host; a cell configured editable again is released from the hold; a cell the host disables again before the microtask is left alone; a control that is not a FormControl gets no hook, and a control is hooked once however often it is configured. A second spec stands the table up under real formly (FormlyModule.forRoot, a form, the builder row template formly-jsonschema produces, nz-input cells as in the app): the rows build with Name and Type locked and Value editable; a wholesale form.enable(), as the panel does, leaves Name and Type locked, read from the inputs' disabled state and not only from the controls'; a read-only form.disable() locks everything and a later enable() gives only the Value cell back. Each guard was deletion-checked and turned a named test red. eslint, prettier and the production (AOT) build pass; the full frontend suite passes; every statement, function and branch this PR adds is covered.

Checked on a local instance (a Python source feeding a Python UDF whose code declares two UI parameters), the panel widened so the columns are in view. On main the cells are editable on most openings (4 of 5) and after every run; with this PR they stayed locked on every opening and through a forced setInteractivity(true) followed by real change-detection passes, read from the DOM as well as from the controls. The Value cell follows the host in both.

Video

Before

Screen.Recording.2026-10-04.at.10.40.48.PM.mov

After

Screen.Recording.2026-10-04.at.10.44.45.PM.mov

Was this PR authored or co-authored using generative AI tooling?

Yes. Generated-by: Claude Code (Anthropic). Co-authored with Claude, reviewed line by line by the author before submission.

🤖 Generated with Claude Code

@github-actions github-actions Bot added fix frontend Changes related to the frontend GUI labels Oct 5, 2026
@Yicong-Huang Yicong-Huang added release/v1.3 back porting to release/v1.3 release/v1.2 back porting to release/v1.2 labels Oct 5, 2026
@github-actions

github-actions Bot commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

Automated Reviewer Suggestions

Based on the git blame history of the changed files, we recommend the following reviewers:

  • Contributors with relevant context: @aicam
    You can notify them by mentioning @aicam in a comment.

@github-actions
github-actions Bot requested review from mengw15 and xuang7 October 5, 2026 05:15
@github-actions

github-actions Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Backport auto-label report

This fix: PR was checked against each actively-supported release branch. A release/* label nominates a backport target; the branch's release manager approving this PR is what sends the fix there. The required Backport Approvals check stays red until every label below is approved, so each manager either approves or removes their own label — which is why the labels left on a merged PR are exactly the branches it reached.

Release branch Analysis
✅ release/v1.3 Already labeled — this fix is queued to backport here. @mengw15 decides: approving sends the fix here, removing this label declines it. The merge waits on one or the other.
✅ release/v1.2 Already labeled — this fix is queued to backport here. @xuang7 decides: approving sends the fix here, removing this label declines it. The merge waits on one or the other.

Auto-label run.

@aicam
aicam enabled auto-merge October 6, 2026 01:39
@yangzhang75

Copy link
Copy Markdown
Contributor Author

@aicam could you merge this when you find some time?

auto-merge was automatically disabled October 17, 2026 00:25

Head branch was pushed to by a user without write access

@yangzhang75
yangzhang75 force-pushed the fix/udf-parameters-lock-under-panel-enable branch from b245c6c to 448c3e3 Compare October 17, 2026 00:25
@codecov-commenter

codecov-commenter commented Oct 17, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.33%. Comparing base (3726dd7) to head (c7d9882).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #8848   +/-   ##
=========================================
  Coverage     92.33%   92.33%           
  Complexity     5045     5045           
=========================================
  Files          1260     1260           
  Lines         54067    54082   +15     
  Branches       6755     6759    +4     
=========================================
+ Hits          49921    49938   +17     
  Misses         2486     2486           
+ Partials       1660     1658    -2     
Flag Coverage Δ *Carryforward flag
access-control-service 77.38% <ø> (ø) Carriedforward from 3726dd7
agent-service 99.16% <ø> (ø) Carriedforward from 3726dd7
amber 87.31% <ø> (ø) Carriedforward from 3726dd7
computing-unit-managing-service 60.48% <ø> (ø) Carriedforward from 3726dd7
config-service 87.37% <ø> (ø) Carriedforward from 3726dd7
file-service 82.00% <ø> (ø) Carriedforward from 3726dd7
frontend 96.56% <100.00%> (+0.01%) ⬆️
notebook-migration-service 83.73% <ø> (ø) Carriedforward from 3726dd7
pyamber 98.57% <ø> (ø) Carriedforward from 3726dd7
workflow-compiling-service 72.43% <ø> (ø) Carriedforward from 3726dd7

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…lls locked when the panel enables its form group

The cells are inferred from the operator's `self.UiParameter(...)` declarations,
so the widget disables them as it builds each row. The operator property panel
then enables its whole form group (setInteractivity), one task after every render
and again whenever the workflow becomes modifiable, and formly mirrors the new
state back into props.disabled, so the lock was simply undone: on main the cells
were editable on 4 of 5 panel openings, and always after a run.

The widget now registers Angular's disabled-change hook on each cell it locks and
disables the control again in the microtask after a host's enable, announcing the
status change. Both details are needed: the input's value accessor is a later hook
on the same control, and ng-zorro's nz-input paints its disabled attribute from
the control's status changes. A cell configured editable again is released.
@yangzhang75
yangzhang75 force-pushed the fix/udf-parameters-lock-under-panel-enable branch from 448c3e3 to c7d9882 Compare October 19, 2026 00:00
@mengw15 mengw15 removed release/v1.2 back porting to release/v1.2 release/v1.3 back porting to release/v1.3 labels Oct 19, 2026
@mengw15
mengw15 removed request for mengw15 and xuang7 October 19, 2026 22:21
@aicam
aicam added this pull request to the merge queue Oct 22, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to no response for status checks Oct 22, 2026
@aicam
aicam added this pull request to the merge queue Nov 4, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Nov 4, 2026
@yangzhang75
yangzhang75 requested a review from aicam November 5, 2026 09:27
@aicam
aicam added this pull request to the merge queue Nov 5, 2026
Merged via the queue into apache:main with commit 8b0fb16 Nov 5, 2026
42 of 53 checks passed
renovate-bot pushed a commit to renovate-bot/apache-_-texera that referenced this pull request Nov 6, 2026
### What changes were proposed in this PR?

Nothing can merge into main right now. Every job that sets up sbt
(`amber`, `amber-integration`, `platform` ×6, `platform-integration` ×6,
`Bench`, the image builds and the dependency graph) has died at `Setup
sbt launcher` since 2026-11-05 15:36 UTC, on main itself and on every
PR, so `Required Checks` is red and the merge queue takes nothing.
Nothing in this repository changed: main at `312a0cefc` (apache#9325, run at
07:16 UTC) was green, main at `8b0fb168e` (apache#8848, run at 20:54 UTC) is
red, and that commit touches three frontend files.

Before: Scala job -> sbt/setup-sbt v1.5.12 -> "Verify the sbt
distribution signature" reads carabiner-dev/policies at HEAD -> ampel
v1.3.6 cannot parse it -> exit 1 before sbt is installed
After: Scala job -> sbt/setup-sbt v1.5.14 -> the same step reads the
policy pinned at carabiner-dev/policies@971d670e9 -> sbt installs -> the
job runs

Root cause. v1.5.12 hands `carabiner-dev/actions/ampel/verify@32587e8`
(ampel v1.3.6) the policy argument
`git+https://github.com/carabiner-dev/policies#signature/signature.json`,
with no ref, so the policy is whatever that repository's main holds at
run time. On 2026-11-05 `signature/signature.json` gained a `meta.name`
field that ampel v1.3.6 rejects, and ampel exits while loading the
policy with `compiling policy: unable to parse a policy, group or
policySet from data`. The step's `fail: false` covers a failed
verification verdict, not the loader exiting, so the composite action
fails and the job stops. sbt/setup-sbt#155, released as v1.5.14 the same
day, pins the policy to
`carabiner-dev/policies@971d670e93f2fb4da304b0e5f071066dc4686a5a`, a
commit without the field.

Change. All 9 call sites (`build.yml` ×4, `build-and-push-images.yml`
×3, `benchmarks.yml`, `dependency-graph.yml`) move from
`ecad189dd245444b9ae0a0cd5922fd938b88906f` (v1.5.12) to
`67d461eba2c4753196c230e02da850757ba4775e` (v1.5.14), as apache#9100 moved
them to v1.5.12. Between the two versions `action.yml` differs in two
lines: the policy pin above and the default `sbt-runner-version` (2.0.10
-> 2.0.12), which this repository does not use because
`project/build.properties` sets `sbt.version`. The nested actions,
`actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9` and
`carabiner-dev/actions/ampel/verify@32587e82f960d49b36101e8c45d1956e511965d3`,
are the same in both, and no call site passes inputs.

Scope. v1.5.13 is already on the ASF allow list but carries the same
unpinned policy line, so it fails the same way; v1.5.14 is the first
version that does not. This PR changes nothing about what the signature
step does when it runs; it only stops the step from reading a moving
target.

**Blocked on apache/infrastructure-actions#1498.**
`sbt/setup-sbt@67d461eba` is not on the ASF actions allow list yet;
apache#1498 (opened 2026-11-05 13:39 UTC) adds it. Until it merges, every sbt
job on this PR fails at `Set up job` with the action not allowed, which
is how v1.5.7 failed in apache#9100, so the red checks here are expected and
this PR must not be merged before apache#1498 lands. Once it does, a re-run of
the failed jobs is the proof; nothing in this branch needs to change.

### Any related issues, documentation, discussions?

Closes apache#9345.

Upstream fix: sbt/setup-sbt#155 (v1.5.14). Allow-list entry:
apache/infrastructure-actions#1498. Precedent: apache#9100 moved the same 9
call sites to v1.5.12 after the ASF allow list dropped the actions
v1.5.7 reached, with backports apache#9132 (v1.3) and apache#9133 (v1.2); apache#7710 and
apache#6990 are the earlier bumps of the same kind.

### How was this PR tested?

Every changed file parses:

```
python -I -c "import sys, yaml; [yaml.safe_load(open(f, encoding='utf-8')) for f in sys.argv[1:]]" .github/workflows/build.yml .github/workflows/build-and-push-images.yml .github/workflows/benchmarks.yml .github/workflows/dependency-graph.yml
```

`git ls-remote --tags https://github.com/sbt/setup-sbt.git` resolves
`refs/tags/v1.5.14^{}` to `67d461eba2c4753196c230e02da850757ba4775e`.
Diffing `action.yml` at `ecad189` against `67d461eba` shows only the two
lines described above, with identical `uses:` lines. Walking the `uses:`
chain under v1.5.14 reaches `ampel/verify@32587e8`, which is in
`apache/infrastructure-actions` `approved_patterns.yml`;
`sbt/setup-sbt@67d461eba` itself is not yet, which
apache/infrastructure-actions#1498 adds.

The rest is CI. The labeler applies `ci` for `.github/**`, which runs
every stack that sets up sbt. Those jobs cannot pass until
apache/infrastructure-actions#1498 lands (see above); the re-run after
that is the test, and the checks on this PR are the first run of v1.5.14
in this repository.

### Was this PR authored or co-authored using generative AI tooling?

Yes. Generated-by: Claude Code (claude-fable-5-1). Co-authored with
Claude; the author reviewed the change before submission.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix frontend Changes related to the frontend GUI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A Python UDF's Name and Type parameter cells come back editable on the operator property panel

5 participants