openlineage: add extractors for python and bash operators #30713

mobuchowski · 2023-04-18T14:33:51Z

Add extractor support for PythonOperator and BashOperator. It's a basic support, adding OL facets to them. It does not yet support advanced features that we plan in the future, like extracting data from taskflow-style jobs.

Additionally, slightly refactor registering extractors.

potiuk

Hmm. I was under the impression that we can get rid of the extractors and make the open-lineage facet extraction embedded in the operators ?

mobuchowski · 2023-04-24T09:41:01Z

Hey @potiuk - as I wrote in the implementation doc we want to migrate them, as we already have them it in OL. Then move to gradually reimplementing this in-operator. This would allow us to release apache-airflow-provider-openlineage with parity to openlineage-airflow, so that users moving to Airflow 2.6 could migrate seamlessly.

potiuk · 2023-04-24T09:49:50Z

Hey @potiuk - as I wrote in the implementation doc we want to migrate them, as we already have them it in OL. Then move to gradually reimplementing this in-operator. This would allow us to release apache-airflow-provider-openlineage with parity to openlineage-airflow, so that users moving to Airflow 2.6 could migrate seamlessly.

Ah. OK makes sense. Can we mark all of those classes as :meta private: (not sure if this is the right way to mark the whole modules as private) and add description in those modules/classes so that it is clear those are private parts and not belonging to the API of the provider ?

mobuchowski · 2023-04-24T12:06:19Z

Ideally I'd add :meta private: to whole airflow.providers.openlineage.extractors package.

potiuk · 2023-04-24T14:56:03Z

Ideally I'd add :meta private: to whole airflow.providers.openlineage.extractors package.

yep

potiuk · 2023-04-24T14:57:12Z

I think we should also (discussed it with @eladkal - add a mechanism - similar to suspended but only for releases) to exclude open-lineage from releasing until it is ready.

eladkal · 2023-04-24T15:21:33Z

I think we should also (discussed it with @eladkal - add a mechanism - similar to suspended but only for releases) to exclude open-lineage from releasing until it is ready.

yeah the manual removal of the provider from the release is not so nice but I guess this is really an edge case... it's unlikely we will encounter another case of merged new provider that we don't want to release so I'm not sure if worth the effort setting this option

potiuk · 2023-04-24T15:39:27Z

I think we should also (discussed it with @eladkal - add a mechanism - similar to suspended but only for releases) to exclude open-lineage from releasing until it is ready.

yeah the manual removal of the provider from the release is not so nice but I guess this is really an edge case... it's unlikely we will encounter another case of merged new provider that we don't want to release so I'm not sure if worth the effort setting this option

Indeed. That's why I just HARDCODED It for now : #30841

mobuchowski · 2023-04-24T16:21:12Z

Thanks @eladkal @potiuk - that makes sense

mobuchowski · 2023-04-26T13:58:24Z

@potiuk @eladkal anything still needed to merge it?

josh-fell · 2023-04-27T01:23:17Z

Just curious, should this provider be AIP-21 compliant? If so, maybe we have file paths of airflow/providers/openlineage/extractors/bash.py and airflow/providers/openlineage/extractors/python.py instead?

mobuchowski · 2023-04-27T09:13:14Z

@josh-fell generally, those wouldn't be used by users of Airflow - you don't need to manually specify those paths anywhere, unless you want to add your own custom extractor, so I personally don't think it matters. I like shorter paths though and I'll change it.

josh-fell · 2023-04-28T01:35:29Z

@josh-fell generally, those wouldn't be used by users of Airflow - you don't need to manually specify those paths anywhere, unless you want to add your own custom extractor, so I personally don't think it matters. I like shorter paths though and I'll change it.

Of course. This was more of a philosophical question since this might not be considered a "traditional" provider so should the AIP apply. I'm a fan because it at least removes some schematic redundancy in the import path, but figured I'd pose the question.

airflow/providers/openlineage/extractors/python.py

ashb · 2023-05-04T08:44:03Z

airflow/providers/openlineage/extractors/bash.py

+        return ["BashOperator"]
+
+    def extract(self) -> OperatorLineage | None:
+        collect_source = os.environ.get("OPENLINEAGE_AIRFLOW_DISABLE_SOURCE_CODE", "true").lower() not in (


Should be from airflow conf now?

Rebased, moved to Airflow conf with env fallback.

ashb · 2023-05-04T08:45:08Z

airflow/providers/openlineage/extractors/manager.py

+            try_import_from_string("airflow.providers.openlineage.extractors.python.PythonExtractor"),
+            try_import_from_string("airflow.providers.openlineage.extractors.bash.BashExtractor"),


Given these are modules from the same provider I don't see how these could fail?

Yeah - moved those to regular imports.

mobuchowski · 2023-05-10T09:35:22Z

@potiuk @josh-fell @pierrejeambrun can you merge?

ashb · 2023-05-10T10:31:40Z

airflow/providers/openlineage/extractors/bash.py

+    @classmethod
+    def get_operator_classnames(cls) -> list[str]:
+        return ["BashOperator"]


This is maybe a bigger question, but is there a reason this couldn't be just a property?

Suggested change

@classmethod

def get_operator_classnames(cls) -> list[str]:

return ["BashOperator"]

operator_classnames: list[str] = ["BashOperator"]

Could be, but we'd have to keep get_operator_classnames on base extractor for backwards compatibility.

ashb · 2023-05-10T10:33:21Z

airflow/providers/openlineage/extractors/bash.py

+                    unknownItems=[
+                        UnknownOperatorInstance(
+                            name="BashOperator",
+                            properties={attr: value for attr, value in self.operator.__dict__.items()},


This is going to include the whole dag. (since self.operator.dag is a property.) Do you want that?

ashb

A few nits, and a question about what we include in the facet (I think it pulls in the dag) -- I'm not sure we should include all attrbibutes of the operator

airflow/providers/openlineage/utils/utils.py

ashb · 2023-05-10T10:38:45Z

airflow/providers/openlineage/utils/utils.py

+    source_var = conf.get(
+        "openlinege", "disable_source_code", fallback=os.getenv("OPENLINEAGE_AIRFLOW_DISABLE_SOURCE_CODE")
+    )
+    return isinstance(source_var, str) and source_var.lower() not in ("true", "1", "t")


Suggested change

source_var = conf.get(

"openlinege", "disable_source_code", fallback=os.getenv("OPENLINEAGE_AIRFLOW_DISABLE_SOURCE_CODE")

)

return isinstance(source_var, str) and source_var.lower() not in ("true", "1", "t")

return conf.getboolean(

"openlinege",

"disable_source_code",

fallback=os.getenv("OPENLINEAGE_AIRFLOW_DISABLE_SOURCE_CODE", "").lower() not in ("true", "1", "t")

)

Can't do this this way unfortunately, since conf.getboolean needs default value, and default value prevents fallback from working, since it's taken before fallback.

ashb · 2023-05-10T10:40:46Z

tests/providers/openlineage/extractors/test_bash_extractor.py

I don't see any tests that set the airflow config, just the "fallback" env. @conf_vars can help.

@ashb added more tests with @conf_vars

ashb · 2023-05-10T10:46:18Z

I've just noticed something. Wasn't one of the key points of AIP-53 that we would have the facet extraction logic on the operator directly?

OpenLineage support in providers:

As part of this new provider, we define a new optional contract for operators defined in the documentation here: get_openlineage_facets*. Each

Signed-off-by: Maciej Obuchowski <obuchowski.maciej@gmail.com>

mobuchowski · 2023-05-12T15:10:28Z

@ashb Okay, let's merge parts 1 and 2 of the implementation plan together. We'll create new PRs with OL support directly in operators.

For this one it won't work - since it requires support in core Airflow, and that would probably require waiting for 2.7 release.

mobuchowski added the provider:openlineage AIP-53 label Apr 18, 2023

boring-cyborg bot added the area:providers label Apr 18, 2023

mobuchowski mentioned this pull request Apr 18, 2023

openlineage: add base SQL extractor for SQL-based operators #30719

Closed

mobuchowski requested review from ashb, potiuk and eladkal April 19, 2023 14:09

mobuchowski force-pushed the openlineage-base-extractors branch from 96aca6f to 181cea1 Compare April 20, 2023 16:27

potiuk reviewed Apr 23, 2023

View reviewed changes

mobuchowski force-pushed the openlineage-base-extractors branch from 181cea1 to fc9792a Compare April 24, 2023 12:37

mobuchowski mentioned this pull request Apr 24, 2023

openlineage: add GCS extractor #30837

Closed

mobuchowski force-pushed the openlineage-base-extractors branch from fc9792a to 731fff3 Compare April 26, 2023 12:45

potiuk approved these changes Apr 26, 2023

View reviewed changes

mobuchowski force-pushed the openlineage-base-extractors branch from 731fff3 to d14efba Compare April 26, 2023 18:00

This was referenced Apr 26, 2023

openlineage: add (S)FTP extractors #30888

Closed

openlineage: add sagemaker extractors #30890

Closed

pierrejeambrun approved these changes Apr 26, 2023

View reviewed changes

mobuchowski force-pushed the openlineage-base-extractors branch from d14efba to 4817629 Compare April 27, 2023 10:14

josh-fell approved these changes Apr 28, 2023

View reviewed changes

airflow/providers/openlineage/extractors/python.py Outdated Show resolved Hide resolved

mobuchowski force-pushed the openlineage-base-extractors branch 4 times, most recently from 884b628 to f8df58f Compare April 28, 2023 17:40

ashb reviewed May 4, 2023

View reviewed changes

mobuchowski force-pushed the openlineage-base-extractors branch 3 times, most recently from d298c67 to 421b0b2 Compare May 8, 2023 14:03

mobuchowski requested a review from ashb May 8, 2023 15:57

mobuchowski force-pushed the openlineage-base-extractors branch 2 times, most recently from dfa1758 to d736a41 Compare May 9, 2023 14:17

This was referenced May 9, 2023

openlineage: additional default extractor tests #31157

Closed

openlineage: add bigquery extractor #31162

Closed

ashb reviewed May 10, 2023

View reviewed changes

mobuchowski force-pushed the openlineage-base-extractors branch 2 times, most recently from e9c7aa6 to c01af00 Compare May 12, 2023 13:52

openlineage: add extractors for python and bash operators

a77ccb8

Signed-off-by: Maciej Obuchowski <obuchowski.maciej@gmail.com>

mobuchowski force-pushed the openlineage-base-extractors branch from c01af00 to a77ccb8 Compare May 12, 2023 15:00

ashb approved these changes May 12, 2023

View reviewed changes

ashb merged commit 981afe2 into apache:main May 12, 2023

mobuchowski mentioned this pull request May 17, 2023

Adapt OpenLineage extractor framework code to make sure externally defined extractors work #29672

Closed

1 task

ephraimbuddy added this to the Airflow 2.7.0 milestone Jul 6, 2023

ephraimbuddy added the changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) label Jul 6, 2023

ephraimbuddy removed this from the Airflow 2.7.0 milestone Jul 6, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

openlineage: add extractors for python and bash operators #30713

openlineage: add extractors for python and bash operators #30713

mobuchowski commented Apr 18, 2023

potiuk left a comment

mobuchowski commented Apr 24, 2023

potiuk commented Apr 24, 2023

mobuchowski commented Apr 24, 2023 •

edited

Loading

potiuk commented Apr 24, 2023

potiuk commented Apr 24, 2023

eladkal commented Apr 24, 2023

potiuk commented Apr 24, 2023

mobuchowski commented Apr 24, 2023

mobuchowski commented Apr 26, 2023

josh-fell commented Apr 27, 2023

mobuchowski commented Apr 27, 2023

josh-fell commented Apr 28, 2023

ashb May 4, 2023

mobuchowski May 8, 2023

ashb May 4, 2023

mobuchowski May 8, 2023

mobuchowski commented May 10, 2023

ashb May 10, 2023

mobuchowski May 10, 2023

ashb May 10, 2023

ashb left a comment

ashb May 10, 2023

mobuchowski May 12, 2023

ashb May 10, 2023

mobuchowski May 12, 2023

ashb commented May 10, 2023 •

edited

Loading

mobuchowski commented May 12, 2023

		try_import_from_string("airflow.providers.openlineage.extractors.python.PythonExtractor"),
		try_import_from_string("airflow.providers.openlineage.extractors.bash.BashExtractor"),

openlineage: add extractors for python and bash operators #30713

openlineage: add extractors for python and bash operators #30713

Conversation

mobuchowski commented Apr 18, 2023

potiuk left a comment

Choose a reason for hiding this comment

mobuchowski commented Apr 24, 2023

potiuk commented Apr 24, 2023

mobuchowski commented Apr 24, 2023 • edited Loading

potiuk commented Apr 24, 2023

potiuk commented Apr 24, 2023

eladkal commented Apr 24, 2023

potiuk commented Apr 24, 2023

mobuchowski commented Apr 24, 2023

mobuchowski commented Apr 26, 2023

josh-fell commented Apr 27, 2023

mobuchowski commented Apr 27, 2023

josh-fell commented Apr 28, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mobuchowski commented May 10, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ashb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ashb commented May 10, 2023 • edited Loading

mobuchowski commented May 12, 2023

mobuchowski commented Apr 24, 2023 •

edited

Loading

ashb commented May 10, 2023 •

edited

Loading