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 2021/09/23 14:04:07 UTC
[GitHub] [airflow] mobuchowski opened a new pull request #18470: LineageBackend is notified on pre_execute and post fail
mobuchowski opened a new pull request #18470:
URL: https://github.com/apache/airflow/pull/18470
Motivation for this PR is [OpenLineage's](https://openlineage.io/) [Airflow integration.](https://github.com/OpenLineage/OpenLineage/tree/main/integration/airflow)
Currently, LineageBackend is notified only when task succesfully completes.
This PR implements support for two additional methods for `LineageBackend` - to become notified when task starts and when it fails. Those methods are optional, and any implementations of `LineageBackend` should work exactly the same if they won't implement it.
With addition to adding those methods, they are hooked with (AFAIK) appropriate places to do so.
Also, changed loading lineage backend to be lazy.
Now, I think few things are incomplete.
I have no idea what behavior should be for lineage inlets and outlets in context of a failed job - so I just ignored them.
Implements https://github.com/apache/airflow/issues/17984
Signed-off-by: Maciej Obuchowski <ma...@getindata.com>
--
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] ashb commented on a change in pull request #18470: LineageBackend is notified on pre_execute and post fail
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #18470:
URL: https://github.com/apache/airflow/pull/18470#discussion_r714841692
##########
File path: airflow/lineage/__init__.py
##########
@@ -47,20 +47,38 @@ class Metadata:
data: Dict = attr.ib()
-def get_backend() -> Optional[LineageBackend]:
- """Gets the lineage backend if defined in the configs"""
- clazz = conf.getimport("lineage", "backend", fallback=None)
-
- if clazz:
- if not issubclass(clazz, LineageBackend):
- raise TypeError(
- f"Your custom Lineage class `{clazz.__name__}` "
- f"is not a subclass of `{LineageBackend.__name__}`."
- )
+NoBackend = ...
+
+
+class Backend:
+ _backend = None
+
+ @classmethod
+ def get_backend(cls) -> Optional[LineageBackend]:
+ """Gets the lineage backend lazily if defined in the configs"""
+ if cls._backend == NoBackend:
+ return None
+ elif cls._backend:
+ return cls._backend
+
+ clazz = conf.getimport("lineage", "backend", fallback=None)
+ if clazz:
+ if not issubclass(clazz, LineageBackend):
+ raise TypeError(
+ f"Your custom Lineage class `{clazz.__name__}` "
+ f"is not a subclass of `{LineageBackend.__name__}`."
+ )
+ else:
+ cls._backend = clazz()
+ return cls._backend
else:
- return clazz()
+ cls._backend = NoBackend
+ return None
+
- return None
+def get_backend():
+ """Gets the lineage backend lazily if defined in the configs"""
+ return Backend.get_backend()
Review comment:
This change can be made much simpler with `@cache` decorator.
--
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] ashb commented on pull request #18470: LineageBackend is notified on pre_execute and post fail
Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #18470:
URL: https://github.com/apache/airflow/pull/18470#issuecomment-963007005
Yeah, I think we are leaning towards this not being part of the Lineage backend API but somewhere else.
--
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] mobuchowski closed pull request #18470: LineageBackend is notified on pre_execute and post fail
Posted by GitBox <gi...@apache.org>.
mobuchowski closed pull request #18470:
URL: https://github.com/apache/airflow/pull/18470
--
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] github-actions[bot] commented on pull request #18470: LineageBackend is notified on pre_execute and post fail
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #18470:
URL: https://github.com/apache/airflow/pull/18470#issuecomment-962698939
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.
--
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] mobuchowski commented on pull request #18470: LineageBackend is notified on pre_execute and post fail
Posted by GitBox <gi...@apache.org>.
mobuchowski commented on pull request #18470:
URL: https://github.com/apache/airflow/pull/18470#issuecomment-962704510
FYI @potiuk there's larger discussion on the issue: https://github.com/apache/airflow/issues/17984
--
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] ashb commented on a change in pull request #18470: LineageBackend is notified on pre_execute and post fail
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #18470:
URL: https://github.com/apache/airflow/pull/18470#discussion_r714844541
##########
File path: airflow/lineage/backend.py
##########
@@ -25,6 +25,27 @@
class LineageBackend:
"""Sends lineage metadata to a backend"""
+ def send_start(
Review comment:
Maybe it's best to follow the existing pattern, but the `send_` part of this name makes assumptions about what the backend is going.
```suggestion
def on_task_isntance_start(
```
is what I would call this. WDYT?
--
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] mobuchowski commented on a change in pull request #18470: LineageBackend is notified on pre_execute and post fail
Posted by GitBox <gi...@apache.org>.
mobuchowski commented on a change in pull request #18470:
URL: https://github.com/apache/airflow/pull/18470#discussion_r714914966
##########
File path: airflow/lineage/backend.py
##########
@@ -25,6 +25,27 @@
class LineageBackend:
"""Sends lineage metadata to a backend"""
+ def send_start(
Review comment:
I'm okay with this. Also, changed `send_fail` to `on_task_instance_fail`.
--
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] mobuchowski commented on a change in pull request #18470: LineageBackend is notified on pre_execute and post fail
Posted by GitBox <gi...@apache.org>.
mobuchowski commented on a change in pull request #18470:
URL: https://github.com/apache/airflow/pull/18470#discussion_r714914379
##########
File path: airflow/lineage/__init__.py
##########
@@ -47,20 +47,38 @@ class Metadata:
data: Dict = attr.ib()
-def get_backend() -> Optional[LineageBackend]:
- """Gets the lineage backend if defined in the configs"""
- clazz = conf.getimport("lineage", "backend", fallback=None)
-
- if clazz:
- if not issubclass(clazz, LineageBackend):
- raise TypeError(
- f"Your custom Lineage class `{clazz.__name__}` "
- f"is not a subclass of `{LineageBackend.__name__}`."
- )
+NoBackend = ...
+
+
+class Backend:
+ _backend = None
+
+ @classmethod
+ def get_backend(cls) -> Optional[LineageBackend]:
+ """Gets the lineage backend lazily if defined in the configs"""
+ if cls._backend == NoBackend:
+ return None
+ elif cls._backend:
+ return cls._backend
+
+ clazz = conf.getimport("lineage", "backend", fallback=None)
+ if clazz:
+ if not issubclass(clazz, LineageBackend):
+ raise TypeError(
+ f"Your custom Lineage class `{clazz.__name__}` "
+ f"is not a subclass of `{LineageBackend.__name__}`."
+ )
+ else:
+ cls._backend = clazz()
+ return cls._backend
else:
- return clazz()
+ cls._backend = NoBackend
+ return None
+
- return None
+def get_backend():
+ """Gets the lineage backend lazily if defined in the configs"""
+ return Backend.get_backend()
Review comment:
Fixed. Thanks!
--
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 #18470: LineageBackend is notified on pre_execute and post fail
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #18470:
URL: https://github.com/apache/airflow/pull/18470#issuecomment-963028027
> Yeah, I think we are leaning towards this not being part of the Lineage backend API but somewhere else.
Cool. Yeah I saw the discussion - makes sense!
--
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