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 2020/10/28 15:35:08 UTC
[GitHub] [airflow] yuqian90 opened a new pull request #11922: Extend the same keyword arguments callable support in ``PythonOperator`` to ``ExternalTaskSensor``
yuqian90 opened a new pull request #11922:
URL: https://github.com/apache/airflow/pull/11922
`PythonOperator` accepts `python_callable` in this form, i.e. the callable can request any number of named arguments in `context`:
```
def python_callable(execution_date, ds_nodash):
...
```
However, `ExternalTaskSensor` accepts only `execution_date_fn` in this form:
```
def execution_date_fn(execution_date):
...
```
A more recent change #8702 added support for this form as well:
```
def execution_date_fn(execution_date, context):
...
```
However #8702 introduced an unpleasant side-effect. Because it enforces the number of arguments is either 1 or 2, it stops people from using common techniques such as loop variable capturing. E.g. this comment illustrate a problem: https://github.com/apache/airflow/pull/8702/files#r497897460
This PR addresses the problem by making `execution_date_fn` work the same way as `python_callable`. `execution_date_fn` is now much more flexible. It can accept any variable number of keyword arguments from `context`. The change is backward compatible. Existing usage continues to work.
E.g. the following callables are all legitimate execution_date_fn after this PR:
```
def execution_date_fn(dt):
...
def execution_date_fn(execution_date):
...
def execution_date_fn(execution_date, context):
...
def execution_date_fn(execution_date, ds_nodash):
...
def execution_date_fn(execution_date, ds_nodash, dag):
...
```
The callable facility in `PythonOperator` has been refactored into `airflow.utils.helper.make_kwargs_callable`. So callables in other places can be made to work the same way if we wish to do that in the future.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on pull request #11922: Extend the same keyword arguments callable support in PythonOperator to some other sensors/operators
Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #11922:
URL: https://github.com/apache/airflow/pull/11922#issuecomment-724254996
I'm tempted to merge this anyway -- most tests are passing.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil merged pull request #11922: Extend the same keyword arguments callable support in PythonOperator to some other sensors/operators
Posted by GitBox <gi...@apache.org>.
kaxil merged pull request #11922:
URL: https://github.com/apache/airflow/pull/11922
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #11922: Extend the same keyword arguments callable support in PythonOperator to some other sensors/operators
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11922:
URL: https://github.com/apache/airflow/pull/11922#issuecomment-724133153
[The Workflow run](https://github.com/apache/airflow/actions/runs/354325549) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #11922: Extend the same keyword arguments callable support in PythonOperator to some other sensors/operators
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11922:
URL: https://github.com/apache/airflow/pull/11922#issuecomment-724132834
[The Workflow run](https://github.com/apache/airflow/actions/runs/354323279) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #11922: Extend the same keyword arguments callable support in PythonOperator to ExternalTaskSensor
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11922:
URL: https://github.com/apache/airflow/pull/11922#issuecomment-718039825
[The Workflow run](https://github.com/apache/airflow/actions/runs/333992850) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #11922: Extend the same keyword arguments callable support in PythonOperator to some other sensors/operators
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11922:
URL: https://github.com/apache/airflow/pull/11922#issuecomment-724132308
[The Workflow run](https://github.com/apache/airflow/actions/runs/354306484) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #11922: Extend the same keyword arguments callable support in PythonOperator to ExternalTaskSensor
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11922:
URL: https://github.com/apache/airflow/pull/11922#issuecomment-718040488
[The Workflow run](https://github.com/apache/airflow/actions/runs/333996246) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] yuqian90 commented on pull request #11922: Extend the same keyword arguments callable support in PythonOperator to some other sensors/operators
Posted by GitBox <gi...@apache.org>.
yuqian90 commented on pull request #11922:
URL: https://github.com/apache/airflow/pull/11922#issuecomment-721560299
> Can you please rebased your PR on latest Master since we have applied [Black](https://github.com/apache/airflow/commit/4e8f9cc8d02b29c325b8a5a76b4837671bdf5f68) and [PyUpgrade](https://github.com/apache/airflow/commit/8c42cf1b00c90f0d7f11b8a3a455381de8e003c5) on Master.
>
> It will help if your squash your commits into single commit first so that there are less conflicts.
Thanks. I've rebased.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on pull request #11922: Extend the same keyword arguments callable support in PythonOperator to some other sensors/operators
Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #11922:
URL: https://github.com/apache/airflow/pull/11922#issuecomment-721440805
Can you please rebased your PR on latest Master since we have applied [Black](https://github.com/apache/airflow/commit/4e8f9cc8d02b29c325b8a5a76b4837671bdf5f68) and [PyUpgrade](https://github.com/apache/airflow/commit/8c42cf1b00c90f0d7f11b8a3a455381de8e003c5) on Master.
It will help if your squash your commits into single commit first so that there are less conflicts.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] yuqian90 commented on pull request #11922: Extend the same keyword arguments callable support in PythonOperator to some other sensors/operators
Posted by GitBox <gi...@apache.org>.
yuqian90 commented on pull request #11922:
URL: https://github.com/apache/airflow/pull/11922#issuecomment-724349553
Thanks @ashb and @kaxil !
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #11922: Extend the same keyword arguments callable support in PythonOperator to some other sensors/operators
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #11922:
URL: https://github.com/apache/airflow/pull/11922#discussion_r519912623
##########
File path: airflow/utils/helpers.py
##########
@@ -202,3 +202,48 @@ def cross_downstream(*args, **kwargs):
stacklevel=2,
)
return import_string('airflow.models.baseoperator.cross_downstream')(*args, **kwargs)
+
+
+def determine_kwargs(func: Callable, args: Union[Tuple, List], kwargs: Dict) -> Dict:
Review comment:
Lets move this in to airflow.utils.operator_helpers.
I'll make this 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #11922: Extend the same keyword arguments callable support in PythonOperator to some other sensors/operators
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11922:
URL: https://github.com/apache/airflow/pull/11922#issuecomment-724096118
The PR needs to run all tests because it modifies core of Airflow! Please rebase it to latest master or ask committer to re-run 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #11922: Extend the same keyword arguments callable support in PythonOperator to some other sensors/operators
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11922:
URL: https://github.com/apache/airflow/pull/11922#issuecomment-724154585
[The Workflow run](https://github.com/apache/airflow/actions/runs/354409716) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] yuqian90 commented on pull request #11922: Extend the same keyword arguments callable support in PythonOperator to ExternalTaskSensor
Posted by GitBox <gi...@apache.org>.
yuqian90 commented on pull request #11922:
URL: https://github.com/apache/airflow/pull/11922#issuecomment-718018382
@Fokko @ashb @BasPH @mik-laj hope you are interested since you worked on/reviewed #5990.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org