You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2023/01/01 22:38:06 UTC
[GitHub] [airflow] vemikhaylov opened a new pull request, #28667: Fix params passing to TaskFlow tasks run with CLI
vemikhaylov opened a new pull request, #28667:
URL: https://github.com/apache/airflow/pull/28667
Params passed to the TaskFlow tasks using the CLI `airflow tasks test` command aren't worked correctly (see the linked issue for the more details and examples). This PR is to fix it.
closes: #28287
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
--
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
[GitHub] [airflow] vemikhaylov commented on a diff in pull request #28667: Fix params passing to TaskFlow tasks run with CLI
Posted by GitBox <gi...@apache.org>.
vemikhaylov commented on code in PR #28667:
URL: https://github.com/apache/airflow/pull/28667#discussion_r1059803154
##########
airflow/example_dags/example_passing_params_via_test_command.py:
##########
@@ -30,18 +30,15 @@
@task(task_id="run_this")
-def my_py_command(params, test_mode=None, task=None):
Review Comment:
It wasn't clear to me if a specific case was intended to be tested here originally with the usage of the `params` name here, which is also a name of the `task.params` attribute or it's just a coincidence.
I moved it to a simpler structure. If the original one made sense and is needed as well, please let me know, I'll restore it.
--
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
[GitHub] [airflow] uranusjr commented on pull request #28667: Fix params passing to TaskFlow tasks run with CLI
Posted by GitBox <gi...@apache.org>.
uranusjr commented on PR #28667:
URL: https://github.com/apache/airflow/pull/28667#issuecomment-1369297403
I don’t think this is correct. [Params are params](https://airflow.apache.org/docs/apache-airflow/stable/concepts/params.html), and different from arguments to the task function. You can access params in a taskflow task like this:
```python
@task
def my_task(params=None):
# At runtime 'params' holds the params.
```
--
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
[GitHub] [airflow] vemikhaylov commented on a diff in pull request #28667: Fix params passing to TaskFlow tasks run with CLI
Posted by GitBox <gi...@apache.org>.
vemikhaylov commented on code in PR #28667:
URL: https://github.com/apache/airflow/pull/28667#discussion_r1059802850
##########
airflow/cli/commands/task_command.py:
##########
@@ -560,6 +561,9 @@ def task_test(args, dag=None):
if args.task_params:
passed_in_params = json.loads(args.task_params)
task.params.update(passed_in_params)
+ if isinstance(task, DecoratedOperator):
+ task.op_args = []
+ task.op_kwargs.update(passed_in_params)
Review Comment:
I'm not 100% sure that this is the best place to override this. Probably the args should rather be overridden somewhere closer to `execute` (e.g., https://github.com/apache/airflow/blob/main/airflow/operators/python.py#L174).
I lack clear understanding what `params` are / should be used for as opposed to `op_args` and `op_kwargs`, so overriding here looked like a safer change to me.
--
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
[GitHub] [airflow] vemikhaylov commented on a diff in pull request #28667: Fix params passing to TaskFlow tasks run with CLI
Posted by GitBox <gi...@apache.org>.
vemikhaylov commented on code in PR #28667:
URL: https://github.com/apache/airflow/pull/28667#discussion_r1059803154
##########
airflow/example_dags/example_passing_params_via_test_command.py:
##########
@@ -30,18 +30,15 @@
@task(task_id="run_this")
-def my_py_command(params, test_mode=None, task=None):
Review Comment:
It wasn't clear to me if a specific case was intended to be tested here originally with the usage of the `params` name here, which is also a name of the `task.params` attribute or it's just a coincidence.
Moved it to a simpler structure. If the original one made sense and is needed as well, please let me know, I'll restore it.
--
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
[GitHub] [airflow] vemikhaylov commented on pull request #28667: Fix params passing to TaskFlow tasks run with CLI
Posted by GitBox <gi...@apache.org>.
vemikhaylov commented on PR #28667:
URL: https://github.com/apache/airflow/pull/28667#issuecomment-1370254884
Hey @uranusjr and @potiuk! Thank you a lot for the feedback and providing a link to the docs, I've learned something new!
So from what you mentioned above, if I understand it correctly, `params` seems to be a different parallel concept to the tasks args and the current behaviour is correct, right?
Is the issue #28287 not completely correct and not a bug, but rather a feature request then? In other words, do we still want to have a way to pass `args` to a task run with the CLI command `airflow tasks test` (e.g., passing `--task-kwargs`)?
--
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
[GitHub] [airflow] potiuk commented on pull request #28667: Fix params passing to TaskFlow tasks run with CLI
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #28667:
URL: https://github.com/apache/airflow/pull/28667#issuecomment-1369529431
Agree with @uranusjr
--
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
[GitHub] [airflow] potiuk closed pull request #28667: Fix params passing to TaskFlow tasks run with CLI
Posted by GitBox <gi...@apache.org>.
potiuk closed pull request #28667: Fix params passing to TaskFlow tasks run with CLI
URL: https://github.com/apache/airflow/pull/28667
--
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