Make skip of trigger form in UI if no params are defined configurable #33351

jscheffl · 2023-08-12T16:51:16Z

This PR is a hotfix for Airflow 2.7.0rc1 adding a configuration option so that the trigger form on UI can be displayed also for DAGs w/o params defined which was removed in #31583.

How to test?

Start Airflow Webserver on this PR
Check that form still displays (always) when triggering DAG "example_bash_operator" - as this has a param defined
Trigger the example_python_operator and see it starts w/o displaying UI
Stop airflow webserver and start it with AIRFLOW__WEBSERVER__SKIP_TRIGGER_FORM_IF_NO_PARAM=False airflow webserver -d and trigger the example_python_operator - See it displays the JSON entry form (again)

closes: #33344

jscheffl · 2023-08-12T17:58:17Z

To be merged based on vote in https://lists.apache.org/thread/pvvn2hxf03nrjm3l9crhmzw5yls6ftpn

airflow/config_templates/config.yml

airflow/www/views.py

pierrejeambrun · 2023-08-12T18:16:37Z

cc: @ephraimbuddy Can be grouped under Trigger Button - Implement Part 2 of AIP-50 (#31583) in the release note I believe.

pierrejeambrun

more nit :)

airflow/www/views.py

…igurable

potiuk · 2023-08-12T22:13:19Z

Approved, but I think we need a newsfragment, describing essentially:

If you used to use dagrun config dictionary of configurations when you run the DAG, you shoud either:

enable the new param ("show_trigger_form_if_no_params") to bring back old behaviour
add params to your DAG as described in ...

Then I am super-happy

ephraimbuddy · 2023-08-13T07:35:47Z

Approved, but I think we need a newsfragment, describing essentially:

If you used to use dagrun config dictionary of configurations when you run the DAG, you shoud either:

enable the new param ("show_trigger_form_if_no_params") to bring back old behaviour

add params to your DAG as described in ...

Then I am super-happy

Yeah, agree with you

ephraimbuddy · 2023-08-13T17:46:13Z

To be merged based on vote in https://lists.apache.org/thread/pvvn2hxf03nrjm3l9crhmzw5yls6ftpn

Merged since the majority are voting in favour of it. Would revert if there's a change in votes

…#33351) * Make skip of trigger form in UI if no params are defined configurable * Review feedback, remove negating bool * Review feedback, remove negating bool * Add newsfragment (cherry picked from commit c036292)

…apache#33351) * Make skip of trigger form in UI if no params are defined configurable * Review feedback, remove negating bool * Review feedback, remove negating bool * Add newsfragment

@ohaibbq

…iviz/recidiviz-data#29712) ## Description of the change Recidiviz/recidiviz-data#27852 upgraded to `composer-2.6.2-airflow-2.6.3` which failed during the tf step and was reverted in Recidiviz/recidiviz-data#29696 The commits in this PR are as follows -- 1. un-reverts Recidiviz/recidiviz-data#27852 2. upgrades to `composer-2.7.1-airflow-2.7.3`, copying requirements over from [version page](https://cloud.google.com/composer/docs/concepts/versioning/composer-versions#images) 3. airflow pipenv lock workflow 4. removes a few imports and checks since [Recidiviz/recidiviz-data#32731](apache/airflow#32731) opened by our very own @ohaibbq is included in 2.7.1 read through the [release notes](https://airflow.apache.org/docs/apache-airflow/stable/release_notes.html#airflow-2-7-0-2023-08-18), nothing in our usage seems to be deprecated; there are a few things that might be of interest to folks: - _Create metrics to track Scheduled->Queued->Running task state transition times ([Recidiviz/recidiviz-data#30612](apache/airflow#30612 will be nice for us to be able to see this re: k8s spin up time - _Add OpenTelemetry to Airflow ([AIP-49](https://github.com/apache/airflow/pulls?q=is%3Apr+is%3Amerged+label%3AAIP-49+milestone%3A%22Airflow+2.7.0%22))_ - _The trigger UI form is skipped in web UI if no parameters are defined in a DAG ([Recidiviz/recidiviz-data#33351](apache/airflow#33351: not sure if folks run ad-hoc dags using the UI instead of `trigger_state_specific_calculation/ingest_dag` but we need to add `show_trigger_form_if_no_params` if we want to still be able to trigger via the UI would love a second set of eyes around these warning logs that occur after tests run, see [here](https://github.com/Recidiviz/recidiviz-data/actions/runs/9071731702/job/24926024866?pr=29712#step:6:53). also occurred locally but i couldn't isolate the cause of the issue ``` Traceback (most recent call last): File "/opt/hostedtoolcache/Python/3.11.8/x64/lib/python3.11/logging/__init__.py", line 1113, in emit stream.write(msg + self.terminator) ValueError: I/O operation on closed file. Call stack: File "/home/runner/.local/share/virtualenvs/airflow-jCIRjM3K/lib/python3.11/site-packages/sqlalchemy/pool/base.py", line 509, in <lambda> and _finalize_fairy( File "/home/runner/.local/share/virtualenvs/airflow-jCIRjM3K/lib/python3.11/site-packages/sqlalchemy/pool/base.py", line 791, in _finalize_fairy pool.logger.error( Message: 'Exception during reset or similar' Arguments: () --- Logging error --- Traceback (most recent call last): File "/home/runner/.local/share/virtualenvs/airflow-jCIRjM3K/lib/python3.11/site-packages/sqlalchemy/pool/base.py", line 763, in _finalize_fairy fairy._reset(pool, transaction_was_reset) File "/home/runner/.local/share/virtualenvs/airflow-jCIRjM3K/lib/python3.11/site-packages/sqlalchemy/pool/base.py", line 1038, in _reset pool._dialect.do_rollback(self) File "/home/runner/.local/share/virtualenvs/airflow-jCIRjM3K/lib/python3.11/site-packages/sqlalchemy/engine/default.py", line 683, in do_rollback dbapi_connection.rollback() psycopg2.OperationalError: server closed the connection unexpectedly This probably means the server terminated abnormally before or while processing the request. During handling of the above exception, another exception occurred: Traceback (most recent call last): File "/opt/hostedtoolcache/Python/3.11.8/x64/lib/python3.11/logging/__init__.py", line 1113, in emit stream.write(msg + self.terminator) ValueError: I/O operation on closed file. Call stack: File "/home/runner/.local/share/virtualenvs/airflow-jCIRjM3K/lib/python3.11/site-packages/sqlalchemy/pool/base.py", line 509, in <lambda> and _finalize_fairy( File "/home/runner/.local/share/virtualenvs/airflow-jCIRjM3K/lib/python3.11/site-packages/sqlalchemy/pool/base.py", line 791, in _finalize_fairy pool.logger.error( Message: 'Exception during reset or similar' Arguments: () ``` ## Type of change > All pull requests must have at least one of the following labels applied (otherwise the PR will fail): | Label | Description | |----------------------------- |----------------------------------------------------------------------------------------------------------- | | Type: Bug | non-breaking change that fixes an issue | | Type: Feature | non-breaking change that adds functionality | | Type: Breaking Change | fix or feature that would cause existing functionality to not work as expected | | Type: Non-breaking refactor | change addresses some tech debt item or prepares for a later change, but does not change functionality | | Type: Configuration Change | adjusts configuration to achieve some end related to functionality, development, performance, or security | | Type: Dependency Upgrade | upgrades a project dependency - these changes are not included in release notes | ## Related issues Closes Recidiviz/recidiviz-data#29554 ## Checklists ### Development **This box MUST be checked by the submitter prior to merging**: - [x] **Double- and triple-checked that there is no Personally Identifiable Information (PII) being mistakenly added in this pull request** These boxes should be checked by the submitter prior to merging: - [ ] Tests have been written to cover the code changed/added as part of this pull request ### Code review These boxes should be checked by reviewers prior to merging: - [x] This pull request has a descriptive title and information useful to a reviewer - [x] Potential security implications or infrastructural changes have been considered, if relevant --------- Co-authored-by: Helper Bot <helperbot@recidiviz.org> GitOrigin-RevId: 355d050265f490df1779ff773533f5aefd41de6b

Make skip of trigger form in UI if no params are defined configurable

79ec776

jscheffl requested review from ryanahamilton, ashb, bbovenzi and pierrejeambrun as code owners August 12, 2023 16:51

boring-cyborg bot added the area:webserver Webserver related Issues label Aug 12, 2023

jscheffl added the type:improvement Changelog: Improvements label Aug 12, 2023

pierrejeambrun reviewed Aug 12, 2023

View reviewed changes

airflow/config_templates/config.yml Outdated Show resolved Hide resolved

pierrejeambrun reviewed Aug 12, 2023

View reviewed changes

airflow/www/views.py Outdated Show resolved Hide resolved

pierrejeambrun added this to the Airflow 2.7.0 milestone Aug 12, 2023

pierrejeambrun added the type:bug-fix Changelog: Bug Fixes label Aug 12, 2023

Review feedback, remove negating bool

0dd758b

pierrejeambrun removed type:improvement Changelog: Improvements type:bug-fix Changelog: Bug Fixes labels Aug 12, 2023

pierrejeambrun approved these changes Aug 12, 2023

View reviewed changes

airflow/www/views.py Outdated Show resolved Hide resolved

airflow/www/views.py Outdated Show resolved Hide resolved

hussein-awala approved these changes Aug 12, 2023

View reviewed changes

jscheffl and others added 2 commits August 12, 2023 20:22

Review feedback, remove negating bool

3e859a1

Merge branch 'main' into feature/33344-make-skip-of-trigger-form-conf…

e2f694c

…igurable

potiuk approved these changes Aug 12, 2023

View reviewed changes

ephraimbuddy approved these changes Aug 13, 2023

View reviewed changes

Add newsfragment

b9ae079

ephraimbuddy merged commit c036292 into apache:main Aug 13, 2023

jscheffl mentioned this pull request Aug 13, 2023

Hotfix for #33344, force enable of trigger UI also if no params defined #33353

Closed

ephraimbuddy added changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) AIP-50 Trigger DAG UI user Form and removed changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) labels Aug 13, 2023

ephraimbuddy added the changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) label Aug 13, 2023

vatsrahul1001 mentioned this pull request Aug 14, 2023

Setting SHOW_TRIGGER_FORM_IF_NO_PARAMS=True redirecting to trigger form for every DAG instead of giving an option to select as in 2.6.3 #33392

Closed

2 tasks

jscheffl deleted the feature/33344-make-skip-of-trigger-form-configurable branch September 1, 2023 10:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make skip of trigger form in UI if no params are defined configurable #33351

Make skip of trigger form in UI if no params are defined configurable #33351

jscheffl commented Aug 12, 2023 •

edited

Loading

jscheffl commented Aug 12, 2023

pierrejeambrun commented Aug 12, 2023 •

edited

Loading

pierrejeambrun left a comment •

edited

Loading

potiuk commented Aug 12, 2023

ephraimbuddy commented Aug 13, 2023

ephraimbuddy commented Aug 13, 2023 •

edited

Loading

Make skip of trigger form in UI if no params are defined configurable #33351

Make skip of trigger form in UI if no params are defined configurable #33351

Conversation

jscheffl commented Aug 12, 2023 • edited Loading

jscheffl commented Aug 12, 2023

pierrejeambrun commented Aug 12, 2023 • edited Loading

pierrejeambrun left a comment • edited Loading

Choose a reason for hiding this comment

potiuk commented Aug 12, 2023

ephraimbuddy commented Aug 13, 2023

ephraimbuddy commented Aug 13, 2023 • edited Loading

jscheffl commented Aug 12, 2023 •

edited

Loading

pierrejeambrun commented Aug 12, 2023 •

edited

Loading

pierrejeambrun left a comment •

edited

Loading

ephraimbuddy commented Aug 13, 2023 •

edited

Loading