kaxil commented on PR #70298:
URL: https://github.com/apache/airflow/pull/70298#issuecomment-5640187935
Round 5 wrap-up, so everything outstanding is in one place. All of it is
verified against `c2d04a7`.
### Blocking
1. The `defer_for_approval` thread, which I have just widened: it is nine
classes across two providers, not five, and a fix scoped to
`defer_for_approval` leaves the four standard HITL operators wrong.
2. `flex-wrap: wrap` on `.capability-filter-toggles`, which currently
overflows 390px on every provider page that renders it.
### Still open from round 4
The field-count nit. The stronger assertion in `ef2d05c` is an improvement,
but the nit was the count in the name, and at HEAD there are now three
hardcoded copies rather than two: the `discover_classes_from_provider`
docstring ("all 13 Module fields"), `test_has_all_13_fields`, and
`test_all_13_fields_present` with its 13-element `required_fields` list.
`Module` is a `@dataclass`, so `{f.name for f in fields(Module)}` collapses all
three and cannot drift. If you would rather keep the literal, say so and I will
resolve the thread.
### Satisfied, please resolve
These three are collapsed as outdated in the UI, so they are easy to miss,
but they are done:
- The `provider-version.njk` tooltip. HEAD reads "Pass deferrable=True where
the operator exposes it; some defer unconditionally", which is the fix.
- `--color-teal-600`. Gone from `tokens.css`, with no references left in
`main.css`.
- The delegation-shapes thread. The rewrite covers all four shapes. I re-ran
the detector at HEAD: `EksPodOperator`, `GlueCrawlerOperator`, and the
Databricks task and warehouse operators all come back True, and
`DiscordWebhookOperator` stays False.
### Description
Two statements in the body no longer match the code:
- "checks for a `deferrable` constructor parameter, reusing the existing
`get_params_from_class()` machinery". That detector is gone. HEAD walks
`execute()` outward through `self.<helper>()` calls and up the MRO through
`super().execute()`, looking for deferral tokens.
- "14 fields total". `Module` and `ModuleContract` both carry 13 at HEAD,
which is also what both tests assert.
The "1779 modules total, 6 durable, 217 deferrable, 5 with both" line
predates both detector rewrites and will move again once the badge question
lands, so it is worth re-running at the end rather than now. The Databricks
examples still hold; I re-checked the four listed as deferrable-only.
### Not asking for either of these
Listing them so neither surfaces later as a surprise. The `title=` tooltips
on the badges and on the new toggles are not keyboard-reachable, but that is a
pattern this PR inherits rather than introduces. And the filter checkboxes have
the same bfcache and form-restore exposure that `moduleSearch` already has
today.
--
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]
