Adding sensor decorator #22562

mingshi-wang · 2022-03-28T07:02:46Z

Added the @task.sensor decorator to convert a Python function to an instance of the BaseSensorOperator. Example usage of the decorator is:

@task.sensor(poke_interval=60, timeout=3600, mode="poke")
def f() -> PokeReturnValue:
    # implement the condition 
    condition_met = ...
    operator_return_value = ...
    return PokeReturnValue(is_done=condition_met, xcom_value=operator_return_value)

closes: #20323

^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.

uranusjr · 2022-03-28T10:16:44Z

airflow/decorators/__init__.pyi

+    @overload
+    def sensor(self, *, python_callable: Optional[Callable] = None, **kwargs) -> TaskDecorator:
+        """
+        Wraps a Python function into a sensor operator.
+        :param python_callable: decorated function that implements the poke() logics of a sensor operator.
+        """


Individual arguments should be spelled out explicitly (instead of using kwargs) since this file is providing editor autocompletion, and kwargs is mostly useless for autocompletion.

Also, does this make sense?

@sensor # No arguments at all! def func(): return PokeReturnValue(...)

If not, python_callable should be removed since that argument is there to specifically support this use case.

Thanks @uranusjr! I've updated the signature.

The only argument useful for the sensor decorator is "python_callable", but please let me know if there are other arguments that need to be explicit.

@uranusjr I need another +1 to this PR. Could you help take a look again? Thanks!

docs/apache-airflow/tutorial_taskflow_api.rst

potiuk · 2022-03-31T17:31:51Z

LGTM. But we need one other approve to merge it.

github-actions · 2022-03-31T17:33:55Z

The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

uranusjr

Is it still in time for 2.3?

potiuk · 2022-04-09T13:27:35Z

Is it still in time for 2.3?

I'd say yes - but leave it up to @jedcunningham @ephraimbuddy to decide.

potiuk · 2022-04-13T16:59:26Z

@ashb @jedcunningham @ephraimbuddy -> shall we add it to 2.3 ? It's a pretty requested feature and seems ready (but I leave it up to you to decide :).

ephraimbuddy · 2022-04-13T21:40:25Z

Fine with me cc: @jedcunningham

jedcunningham · 2022-04-13T23:38:27Z

No objections here 👍

airflow/example_dags/example_sensor_decorator.py

ashb · 2022-04-14T06:26:37Z

Someone will need to check how this interacts with task mapping first please

potiuk · 2022-04-25T18:35:28Z

I think this one is a bit risky to add for 2.3.0 so let's iterate and merge after we branch off.

abhilash1in · 2022-06-03T16:47:00Z

Eagerly waiting for this to be merged and released!

mingshi-wang · 2022-06-04T16:12:19Z

@potiuk Is it OK to merge this PR?

potiuk · 2022-06-04T16:42:05Z

@mingshi-wang - docs build is failing (that's something that you should fix first) and rebasing the change would also be useful while you do it.

Also we are waiting for @jedcunningham 's re-review - just re-requested it.

potiuk · 2022-09-20T11:59:56Z

I think our Public runners are not big enough to handle "full tests" with k8s :( . So I removed "full tests needed" label and closed/reopened it to run a smaller set of tests

mingshi-wang · 2022-09-20T15:14:37Z

I think our Public runners are not big enough to handle "full tests" with k8s :( . So I removed "full tests needed" label and closed/reopened it to run a smaller set of tests

Thanks for your help!

mingshi-wang · 2022-09-20T15:15:54Z

HI @jedcunningham I have addressed your comments. Can you take a look again? Thanks!

mingshi-wang · 2022-09-21T15:19:56Z

Hi @potiuk will you be ok to merge this PR? I see all tests passed.

potiuk · 2022-09-22T11:09:14Z

We are waiting for @jedcunningham to confirm that he is OK with it - can't merge it without it.

mingshi-wang · 2022-09-25T18:58:38Z

HI @jedcunningham can you please take a look again? Thanks!

mingshi-wang · 2022-10-03T16:41:35Z

We are waiting for @jedcunningham to confirm that he is OK with it - can't merge it without it.

Hi @potiuk, Jed seems unavailable. Is it possible to get another reviewer?

mingshi-wang · 2022-10-09T17:31:42Z

Hi @jedcunningham - do you have a chance to take a look?

mingshi-wang · 2022-10-13T17:15:15Z

Hi @uranusjr @potiuk I think I need some help merging this PR. @jedcunningham is not available to approve this PR. Would you provide some help wrapping up this PR? Thanks!

No response :)

potiuk · 2022-10-23T14:58:55Z

I dismissed Jed's review - can you please rebase it @mingshi-wang (there is a conflict) - sorry for long delay - I've been on holidays :).

mingshi-wang · 2022-10-25T16:34:09Z

I dismissed Jed's review - can you please rebase it @mingshi-wang (there is a conflict) - sorry for long delay - I've been on holidays :).

I rebased the PR. Could you help merge it?

potiuk · 2022-10-26T01:14:08Z

Just one more check @uranusjr @jedcunningham ?

uranusjr · 2022-10-26T06:35:43Z

airflow/decorators/__init__.pyi

+    @overload
+    def sensor(self, **kwargs) -> TaskDecorator:
+        """
+        Wraps a Python function into a sensor operator.
+        """


Can you list out more arguments and the docstring explicitly (like other functions above)? This is shown in editor autocomplete, and the current format is not useful.

potiuk · 2022-10-31T04:02:37Z

@mingshi-wang - I'd love to merge it - can you please address the last comments of @uranusjr ?

mingshi-wang · 2022-10-31T07:21:03Z

@mingshi-wang - I'd love to merge it - can you please address the last comments of @uranusjr ?

Sure will do.

mingshi-wang · 2022-11-01T21:55:52Z

@mingshi-wang - I'd love to merge it - can you please address the last comments of @uranusjr?

@potiuk @uranusjr I addressed the comments. Could you please take a look again?

mingshi-wang · 2022-11-04T17:10:20Z

@mingshi-wang - I'd love to merge it - can you please address the last comments of @uranusjr ?

Hi @potiuk are you ok to merge the PR?

potiuk · 2022-11-07T02:06:37Z

🎉 🎉 - finally @mingshi-wang !

ashb · 2023-03-15T11:29:31Z

airflow/decorators/sensor.py

+from airflow.sensors.python import PythonSensor
+
+
+class DecoratedSensorOperator(PythonSensor):


Oh, this doesn't inherit from DecoratedOperator -- was there any reason for it not?

(Not a problem I guess, since it's all tested etc.)

mingshi-wang requested review from kaxil and potiuk as code owners March 28, 2022 07:02

boring-cyborg bot added the kind:documentation label Mar 28, 2022

mingshi-wang force-pushed the sensor branch from 44eb6c3 to b5c32bd Compare March 28, 2022 07:56

uranusjr reviewed Mar 28, 2022

View reviewed changes

mingshi-wang force-pushed the sensor branch from b5c32bd to 80d642e Compare March 28, 2022 16:20

mingshi-wang requested a review from uranusjr March 28, 2022 17:18

potiuk reviewed Mar 29, 2022

View reviewed changes

docs/apache-airflow/tutorial_taskflow_api.rst Outdated Show resolved Hide resolved

mingshi-wang force-pushed the sensor branch from 80d642e to 394893a Compare March 31, 2022 17:23

potiuk approved these changes Mar 31, 2022

View reviewed changes

github-actions bot added the full tests needed We need to run full set of tests for this PR to merge label Mar 31, 2022

uranusjr approved these changes Apr 9, 2022

View reviewed changes

jedcunningham closed this Apr 14, 2022

jedcunningham reopened this Apr 14, 2022

jedcunningham previously requested changes Apr 14, 2022

View reviewed changes

airflow/example_dags/example_sensor_decorator.py Outdated Show resolved Hide resolved

mingshi-wang force-pushed the sensor branch from 11ae3da to ecf8ad5 Compare April 20, 2022 19:20

potiuk requested a review from jedcunningham June 4, 2022 16:40

mingshi-wang force-pushed the sensor branch from ecf8ad5 to 461e9d4 Compare June 6, 2022 15:06

mingshi-wang force-pushed the sensor branch from 461e9d4 to 32b310f Compare July 1, 2022 15:17

mingshi-wang force-pushed the sensor branch from ce36503 to b7e9c98 Compare October 24, 2022 17:49

uranusjr reviewed Oct 26, 2022

View reviewed changes

potiuk added this to the Airflow 2.5.0 milestone Oct 31, 2022

hu-dabao mentioned this pull request Oct 31, 2022

Add sensor decorator #20530

Closed

mingshi-wang force-pushed the sensor branch 2 times, most recently from 348a45b to 1398b10 Compare November 1, 2022 17:47

Adding sensor decorator

6d391fa

mingshi-wang force-pushed the sensor branch from 1398b10 to 6d391fa Compare November 1, 2022 20:58

uranusjr approved these changes Nov 2, 2022

View reviewed changes

potiuk merged commit cfd63df into apache:main Nov 7, 2022

ephraimbuddy added the type:new-feature Changelog: New Features label Nov 16, 2022

ashb reviewed Mar 15, 2023

View reviewed changes

		from airflow.sensors.python import PythonSensor


		class DecoratedSensorOperator(PythonSensor):

Adding sensor decorator #22562

Adding sensor decorator #22562

Conversation

mingshi-wang commented Mar 28, 2022 • edited Loading

uranusjr Mar 28, 2022 • edited Loading

Choose a reason for hiding this comment

mingshi-wang Mar 28, 2022

Choose a reason for hiding this comment

mingshi-wang Mar 28, 2022

Choose a reason for hiding this comment

mingshi-wang Mar 31, 2022

Choose a reason for hiding this comment

potiuk commented Mar 31, 2022

github-actions bot commented Mar 31, 2022

uranusjr left a comment

Choose a reason for hiding this comment

potiuk commented Apr 9, 2022

potiuk commented Apr 13, 2022

ephraimbuddy commented Apr 13, 2022

jedcunningham commented Apr 13, 2022

ashb commented Apr 14, 2022

potiuk commented Apr 25, 2022

abhilash1in commented Jun 3, 2022

mingshi-wang commented Jun 4, 2022

potiuk commented Jun 4, 2022

potiuk commented Sep 20, 2022

mingshi-wang commented Sep 20, 2022

mingshi-wang commented Sep 20, 2022

mingshi-wang commented Sep 21, 2022

potiuk commented Sep 22, 2022

mingshi-wang commented Sep 25, 2022

mingshi-wang commented Oct 3, 2022

mingshi-wang commented Oct 9, 2022

mingshi-wang commented Oct 13, 2022

potiuk commented Oct 23, 2022 • edited Loading

mingshi-wang commented Oct 25, 2022

potiuk commented Oct 26, 2022

uranusjr Oct 26, 2022

Choose a reason for hiding this comment

potiuk commented Oct 31, 2022

mingshi-wang commented Oct 31, 2022

mingshi-wang commented Nov 1, 2022 • edited by uranusjr Loading

mingshi-wang commented Nov 4, 2022

potiuk commented Nov 7, 2022

ashb Mar 15, 2023

Choose a reason for hiding this comment

ashb Mar 15, 2023

Choose a reason for hiding this comment

mingshi-wang commented Mar 28, 2022 •

edited

Loading

uranusjr Mar 28, 2022 •

edited

Loading

potiuk commented Oct 23, 2022 •

edited

Loading

mingshi-wang commented Nov 1, 2022 •

edited by uranusjr

Loading