You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "DG47 (via GitHub)" <gi...@apache.org> on 2026/10/02 18:21:06 UTC
[PR] Skip Dag param injection for Databricks wheel tasks with positional parameters [airflow]
DG47 opened a new pull request, #74120:
URL: https://github.com/apache/airflow/pull/74120
Since provider 7.16 (#66613) `DatabricksSubmitRunOperator` forwards the operator's `params` into `python_wheel_task.named_parameters` whenever that field is empty. The guard only looked at `named_parameters`, so a Python wheel task that already carries positional `parameters` was submitted with both fields populated. The Databricks `runs/submit` API treats the two as mutually exclusive ("Leave it empty if `named_parameters` is not null") and rejects the run, so adding a Dag-level param to such a Dag breaks every wheel task that uses positional arguments.
**Fix**
* Add `_TASK_PARAM_SLOTS_CONFLICTING_WITH_DICT_PARAM_FIELD` mapping each task type to the parameter slots that cannot coexist with the dict-shaped injection target (currently `python_wheel_task.parameters`), mirroring the existing `_RUN_NOW_PARAM_SLOTS_CONFLICTING_WITH_JOB_PARAMETERS` guard in `DatabricksRunNowOperator`.
* `_inject_airflow_params_into_task` skips injection when one of those slots is populated. An empty `parameters` list still counts as unset, consistent with how an empty `named_parameters` is treated. Other tasks in the same payload are still filled in.
* Docstring and `submit_run.rst` updated to document the skip.
**Testing**
New `test_submit_run_skips_param_injection_when_wheel_task_has_positional_parameters` (parametrized: populated list -> no injection; empty list -> injection) fails on `main` and passes with this change.
```
pytest providers/databricks/tests/unit/databricks/operators/test_databricks.py
267 passed
```
closes: #74095
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Skip Dag param injection for Databricks wheel tasks with positional parameters [airflow]
Posted by "DG47 (via GitHub)" <gi...@apache.org>.
DG47 commented on PR #74120:
URL: https://github.com/apache/airflow/pull/74120#issuecomment-6269936773
Gentle ping on this one when a Databricks provider reviewer has a moment. It fixes #74095: since #66613, `DatabricksSubmitRunOperator` injects the operator's `params` into `python_wheel_task.named_parameters` even when the wheel task already has positional `parameters`, which can't be combined with them. Injection is now skipped in that case. The branch was rebased on 10-13, and the new test fails on main and passes with the change.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Skip Dag param injection for Databricks wheel tasks with positional parameters [airflow]
Posted by "diederikperdok (via GitHub) via commits" <co...@airflow.apache.org>.
diederikperdok commented on PR #74120:
URL: https://github.com/apache/airflow/pull/74120#issuecomment-6576268358
I'd like to kindly ask the maintainers to please also consider the alternative of completely removing the injection of DAG parameters into Airflow tasks. Then the special case added in this PR won't be needed either.
I suspect silently injecting DAG-level parameters into tasks is surprising to users and it makes DAGs brittle: every time a new DAG-level parameter is added it will likely break all Databricks jobs with python wheel tasks.
The [issue that prompted this implementation](https://github.com/apache/airflow/issues/39002) provides as justification that it allows to "Dynamically update job parameter in spark job", but that was already possible using templating, for example like so:
```python
# Inside a DAG which has at least the parameters "environment" and "batch_size"
submit_wheel_task = DatabricksSubmitRunOperator(
task_id="submit_wheel_task",
databricks_conn_id="databricks_default",
run_name="my_wheel_etl_run",
existing_cluster_id="1234-567890-abcde123",
libraries=[{"whl": "dbfs:/FileStore/wheels/my_package-0.1.0-py3-none-any.whl"}],
python_wheel_task={
"package_name": "my_package",
"entry_point": "main",
"parameters": [
"--env", "{{ params.environment }}",
"--batch-size", "{{ params.batch_size }}",
],
},
)
```
In the above example the DAG author has full control over which DAG parameters are dynamically passed on to which Databricks parameters, and no tasks would break if a DAG-level parameter were to be added which should not be passed on to Databricks tasks.
I can't find a discussion on the consequence and pro's and cons of silently injecting all DAG-level parameters into tasks; perhaps this bug is a good cause for reconsideration.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Skip Dag param injection for Databricks wheel tasks with positional parameters [airflow]
Posted by "DG47 (via GitHub)" <gi...@apache.org>.
DG47 commented on PR #74120:
URL: https://github.com/apache/airflow/pull/74120#issuecomment-6448569394
@eladkal could you take a look when you have a moment? It fixes #74095: `DatabricksSubmitRunOperator` injects `params` into `python_wheel_task.named_parameters` even when the wheel task already has positional `parameters`, which can't be combined. The branch is up to date with main and the new test covers both the populated and empty `parameters` cases.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Skip Dag param injection for Databricks wheel tasks with positional parameters [airflow]
Posted by "boring-cyborg[bot] (via GitHub)" <gi...@apache.org>.
boring-cyborg[bot] commented on PR #74120:
URL: https://github.com/apache/airflow/pull/74120#issuecomment-5958633223
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our [Contributors' Guide](https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
Here are some useful points:
- Pay attention to the quality of your code (ruff, mypy and type annotations). Our [prek-hooks]( https://github.com/apache/airflow/blob/main/contributing-docs/08_static_code_checks.rst#prerequisites-for-prek-hooks) will help you with that.
- In case of a new feature add useful documentation (in docstrings or in `docs/` directory). Adding a new operator? Check this short [guide](https://github.com/apache/airflow/blob/main/airflow-core/docs/howto/custom-operator.rst) Consider adding an example Dag that shows how users should use it.
- Consider using [Breeze environment](https://github.com/apache/airflow/blob/main/dev/breeze/doc/README.rst) for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
- Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
- Please follow [ASF Code of Conduct](https://www.apache.org/foundation/policies/conduct) for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
- Be sure to read the [Airflow Coding style]( https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#coding-style-and-best-practices).
- Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits.
Apache Airflow is a community-driven project and together we are making it better 🚀.
In case of doubts contact the developers at:
Mailing List: dev@airflow.apache.org
Slack: https://s.apache.org/airflow-slack
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Skip Dag param injection for Databricks wheel tasks with positional parameters [airflow]
Posted by "DG47 (via GitHub) via commits" <co...@airflow.apache.org>.
DG47 commented on PR #74120:
URL: https://github.com/apache/airflow/pull/74120#issuecomment-6608823222
Thanks @diederikperdok, that's a fair question and worth raising. I'd separate the two, though. This PR fixes a case where the behaviour shipped in 7.16 produces a run the Databricks API rejects outright, so anyone with a Dag-level param and a wheel task using positional `parameters` is broken today regardless of how the wider question is settled. Whether the injection should exist at all is a user-visible behaviour change that would need a deprecation path and a decision from the provider maintainers; if they go that way, this guard simply becomes dead code and can be removed with it. I'd suggest opening that as its own issue or dev-list thread so it gets the discussion it deserves rather than being decided inside a bug fix.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org