You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "sunank200 (via GitHub)" <gi...@apache.org> on 2025/12/30 06:43:43 UTC
[PR] Make sqlalchemy optional dependency for openlineage provider [airflow]
sunank200 opened a new pull request, #59921:
URL: https://github.com/apache/airflow/pull/59921
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
Makes SQLAlchemy an optional dependency for the OpenLineage provider by adding it as an optional extra.
- Wrapped sqlalchemy imports with try/except and raise AirflowOptionalProviderFeatureException when missing
- Users who need SQL lineage features should install: `pip install 'apache-airflow-providers-openlineage[sqlalchemy]'`
closes: [#59904](https://github.com/apache/airflow/issues/59904)
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of an existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
<!-- Please keep an empty line above the dashes. -->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.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 [airflow-core/newsfragments](https://github.com/apache/airflow/tree/main/airflow-core/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
Re: [PR] Make sqlalchemy optional dependency for openlineage provider [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #59921:
URL: https://github.com/apache/airflow/pull/59921#discussion_r2655447653
##########
providers/openlineage/src/airflow/providers/openlineage/utils/utils.py:
##########
@@ -537,7 +538,13 @@ def is_selective_lineage_enabled(obj: DAG | SerializedDAG | AnyOperator) -> bool
@provide_session
def is_ti_rescheduled_already(ti: TaskInstance, session=NEW_SESSION):
- from sqlalchemy import exists, select
+ try:
Review Comment:
Ups :). So yeah .. It actually found an issue :)
--
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
Re: [PR] Make sqlalchemy optional dependency for openlineage provider [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #59921:
URL: https://github.com/apache/airflow/pull/59921#issuecomment-3700663762
> One more note, in utils.py we're also using some db methods like `DagRun.fetch_task_instances` [here](https://github.com/apache/airflow/blob/cf80ae19840f1d03e16323e7bca819550633db97/providers/openlineage/src/airflow/providers/openlineage/utils/utils.py#L988), but we only call it on scheduler. We're also going to add another call to db [here](https://github.com/apache/airflow/pull/59521/changes#diff-090bdefe8e3de4ab44c24490d0b0df9ff2f9445d2d42126e0ce3476908b804d5R1075) for asset triggered dagruns, also performed only on scheduler. Not sure how this relates to treating sqlalchemy as optional.
Not yet. This will come later when we will split (some) providers into two types of providers (maybe they won't be named like that) - we've been disussing it but deferred decision on that to after we complete task isolation and separated core and task-sdk. We iwll have to separate some distributions in two:
* executors/ui plugins/scheduler listeners and plugins (installable with airflow-core)
* Hooks/Operators/macro plugins etc. (installable with task-sdk)
This falls in the same part - part of the current openlineage provider should be a distribution installed with "task-sdk" and part should be another distribution installed with "airflow-core".
Later we can even split "airflow-ui" as separate distribution and have 3 types of "providers"
--
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
Re: [PR] Make sqlalchemy optional dependency for openlineage provider [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #59921:
URL: https://github.com/apache/airflow/pull/59921#discussion_r2652892987
##########
providers/openlineage/src/airflow/providers/openlineage/utils/utils.py:
##########
@@ -537,7 +538,13 @@ def is_selective_lineage_enabled(obj: DAG | SerializedDAG | AnyOperator) -> bool
@provide_session
def is_ti_rescheduled_already(ti: TaskInstance, session=NEW_SESSION):
- from sqlalchemy import exists, select
+ try:
Review Comment:
I think that one indicates an issue -> from what I read that method is called in the plugin in `on_task_instance_running`, which .... will not have the session at all.
```
| 3.0.0 | ``on_task_instance_running`` | ``session`` argument removed from task instance listeners, |
| | | ``task_instance`` object is now an instance of ``RuntimeTaskInstance`` |
```
So I'd say this method is not Airflow 3 compatible ?
--
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
Re: [PR] Make sqlalchemy optional dependency for openlineage provider [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #59921:
URL: https://github.com/apache/airflow/pull/59921
--
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
Re: [PR] Make sqlalchemy optional dependency for openlineage provider [airflow]
Posted by "sunank200 (via GitHub)" <gi...@apache.org>.
sunank200 commented on code in PR #59921:
URL: https://github.com/apache/airflow/pull/59921#discussion_r2655384966
##########
providers/openlineage/src/airflow/providers/openlineage/utils/sql.py:
##########
@@ -217,6 +235,11 @@ def create_filter_clauses(
therefore it is expected the table has them defined.
:param uppercase_names: if True use schema and table names uppercase
"""
+ if and_ is None or or_ is None:
+ raise AirflowOptionalProviderFeatureException(
+ "sqlalchemy is required for SQL filter clause generation. "
+ "Install it with: pip install 'apache-airflow-providers-openlineage[sqlalchemy]'"
+ )
Review Comment:
Changed it
##########
providers/openlineage/src/airflow/providers/openlineage/utils/sql.py:
##########
@@ -157,6 +163,18 @@ def create_information_schema_query(
sqlalchemy_engine: Engine | None = None,
) -> str:
"""Create query for getting table schemas from information schema."""
+ if (
+ Column is None
+ or MetaData is None
+ or Table is None
+ or and_ is None
+ or or_ is None
+ or union_all is None
+ ):
+ raise AirflowOptionalProviderFeatureException(
+ "sqlalchemy is required for SQL schema query generation. "
+ "Install it with: pip install 'apache-airflow-providers-openlineage[sqlalchemy]'"
+ )
Review Comment:
this is better.
--
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
Re: [PR] Make sqlalchemy optional dependency for openlineage provider [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #59921:
URL: https://github.com/apache/airflow/pull/59921#discussion_r2652959141
##########
providers/openlineage/pyproject.toml:
##########
@@ -66,6 +66,13 @@ dependencies = [
"openlineage-python>=1.41.0",
]
+# The optional dependencies should be modified in place in the generated file
+# Any change in the dependencies is preserved when the file is regenerated
+[project.optional-dependencies]
+"sqlalchemy" = [
+ "sqlalchemy[asyncio]>=1.4.49",
Review Comment:
I think we do not really need asyncio here :)
--
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
Re: [PR] Make sqlalchemy optional dependency for openlineage provider [airflow]
Posted by "kacpermuda (via GitHub)" <gi...@apache.org>.
kacpermuda commented on code in PR #59921:
URL: https://github.com/apache/airflow/pull/59921#discussion_r2652921013
##########
providers/openlineage/src/airflow/providers/openlineage/sqlparser.py:
##########
@@ -497,6 +498,14 @@ def get_openlineage_facets_with_sql(
try:
sqlalchemy_engine = hook.get_sqlalchemy_engine()
+ except ImportError as e:
+ if "sqlalchemy" in str(e).lower() or "No module named 'sqlalchemy" in str(e):
+ raise AirflowOptionalProviderFeatureException(
Review Comment:
Yes, I don't think we need another except here for ImportError, the generic one we have below will do just fine
##########
providers/openlineage/src/airflow/providers/openlineage/utils/sql.py:
##########
@@ -157,6 +163,18 @@ def create_information_schema_query(
sqlalchemy_engine: Engine | None = None,
) -> str:
"""Create query for getting table schemas from information schema."""
+ if (
+ Column is None
+ or MetaData is None
+ or Table is None
+ or and_ is None
+ or or_ is None
+ or union_all is None
+ ):
+ raise AirflowOptionalProviderFeatureException(
+ "sqlalchemy is required for SQL schema query generation. "
+ "Install it with: pip install 'apache-airflow-providers-openlineage[sqlalchemy]'"
+ )
Review Comment:
```suggestion
try:
from sqlalchemy import Column, MetaData, Table, and_, or_, union_all
except ImportError:
raise AirflowOptionalProviderFeatureException(
"sqlalchemy is required for SQL schema query generation. "
"Install it with: pip install 'apache-airflow-providers-openlineage[sqlalchemy]'"
)
```
We need this import in only two places, we can probably go with local imports? Both are fine for me, just nit.
##########
providers/openlineage/src/airflow/providers/openlineage/utils/sql.py:
##########
@@ -217,6 +235,11 @@ def create_filter_clauses(
therefore it is expected the table has them defined.
:param uppercase_names: if True use schema and table names uppercase
"""
+ if and_ is None or or_ is None:
+ raise AirflowOptionalProviderFeatureException(
+ "sqlalchemy is required for SQL filter clause generation. "
+ "Install it with: pip install 'apache-airflow-providers-openlineage[sqlalchemy]'"
+ )
Review Comment:
```suggestion
try:
from sqlalchemy import and_, or_
except ImportError:
raise AirflowOptionalProviderFeatureException(
"sqlalchemy is required for SQL filter clause generation. "
"Install it with: pip install 'apache-airflow-providers-openlineage[sqlalchemy]'"
)
```
Same as above
##########
providers/openlineage/src/airflow/providers/openlineage/utils/utils.py:
##########
@@ -537,7 +538,13 @@ def is_selective_lineage_enabled(obj: DAG | SerializedDAG | AnyOperator) -> bool
@provide_session
def is_ti_rescheduled_already(ti: TaskInstance, session=NEW_SESSION):
- from sqlalchemy import exists, select
+ try:
Review Comment:
Just above this snippet, in line 536 you can see that this function is available only for AF2 (hidden behind `if`), so we should be good.
--
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
Re: [PR] Make sqlalchemy optional dependency for openlineage provider [airflow]
Posted by "kacpermuda (via GitHub)" <gi...@apache.org>.
kacpermuda commented on PR #59921:
URL: https://github.com/apache/airflow/pull/59921#issuecomment-3699246599
One more note, in utils.py we're also using some db methods like `DagRun.fetch_task_instances` [here](https://github.com/apache/airflow/blob/cf80ae19840f1d03e16323e7bca819550633db97/providers/openlineage/src/airflow/providers/openlineage/utils/utils.py#L988), but we only call it on scheduler. We're also going to add another call to db [here](https://github.com/apache/airflow/pull/59521/changes#diff-090bdefe8e3de4ab44c24490d0b0df9ff2f9445d2d42126e0ce3476908b804d5R1075) for asset triggered dagruns, also performed only on scheduler. Not sure how this relates to treating sqlalchemy as optional.
--
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
Re: [PR] Make sqlalchemy optional dependency for openlineage provider [airflow]
Posted by "sunank200 (via GitHub)" <gi...@apache.org>.
sunank200 commented on code in PR #59921:
URL: https://github.com/apache/airflow/pull/59921#discussion_r2655422718
##########
providers/openlineage/src/airflow/providers/openlineage/utils/utils.py:
##########
@@ -537,7 +538,13 @@ def is_selective_lineage_enabled(obj: DAG | SerializedDAG | AnyOperator) -> bool
@provide_session
def is_ti_rescheduled_already(ti: TaskInstance, session=NEW_SESSION):
- from sqlalchemy import exists, select
+ try:
Review Comment:
The function is already guarded by 'if not AIRFLOW_V_3_0_PLUS:' so the inner check for AIRFLOW_V_3_0_PLUS was unreachable dead code. Removed the dead code.
--
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
Re: [PR] Make sqlalchemy optional dependency for openlineage provider [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #59921:
URL: https://github.com/apache/airflow/pull/59921#issuecomment-3702265529
Nice!
--
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
Re: [PR] Make sqlalchemy optional dependency for openlineage provider [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #59921:
URL: https://github.com/apache/airflow/pull/59921#discussion_r2652901240
##########
providers/openlineage/src/airflow/providers/openlineage/sqlparser.py:
##########
@@ -497,6 +498,14 @@ def get_openlineage_facets_with_sql(
try:
sqlalchemy_engine = hook.get_sqlalchemy_engine()
+ except ImportError as e:
+ if "sqlalchemy" in str(e).lower() or "No module named 'sqlalchemy" in str(e):
+ raise AirflowOptionalProviderFeatureException(
Review Comment:
I think here we should not raise the exception, but we should raise warning, I think this method should work fine without sqlalchemy - if it can get information from sql_parser (which also does not need sqlalchemy ?)
--
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
Re: [PR] Make sqlalchemy optional dependency for openlineage provider [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #59921:
URL: https://github.com/apache/airflow/pull/59921#discussion_r2652894297
##########
providers/openlineage/src/airflow/providers/openlineage/utils/utils.py:
##########
@@ -537,7 +538,13 @@ def is_selective_lineage_enabled(obj: DAG | SerializedDAG | AnyOperator) -> bool
@provide_session
def is_ti_rescheduled_already(ti: TaskInstance, session=NEW_SESSION):
- from sqlalchemy import exists, select
+ try:
Review Comment:
The `@provide_session` might mask the problem but it still needs to have credentials and connect to the DB of Airflow which should **not** be possible.
cc: @mobuchowski @kacpermuda ?
--
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
Re: [PR] Make sqlalchemy optional dependency for openlineage provider [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #59921:
URL: https://github.com/apache/airflow/pull/59921#issuecomment-3699201919
Nice - few questions - but more to openlineage Airflow 3 integration :)
--
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
Re: [PR] Make sqlalchemy optional dependency for openlineage provider [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #59921:
URL: https://github.com/apache/airflow/pull/59921#discussion_r2652958573
##########
providers/openlineage/pyproject.toml:
##########
@@ -66,6 +66,13 @@ dependencies = [
"openlineage-python>=1.41.0",
]
+# The optional dependencies should be modified in place in the generated file
+# Any change in the dependencies is preserved when the file is regenerated
+[project.optional-dependencies]
+"sqlalchemy" = [
+ "sqlalchemy[asyncio]>=1.4.49",
Review Comment:
```suggestion
"sqlalchemy>=1.4.49",
```
--
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
Re: [PR] Make sqlalchemy optional dependency for openlineage provider [airflow]
Posted by "sunank200 (via GitHub)" <gi...@apache.org>.
sunank200 commented on code in PR #59921:
URL: https://github.com/apache/airflow/pull/59921#discussion_r2655387343
##########
providers/openlineage/src/airflow/providers/openlineage/utils/sql.py:
##########
@@ -157,6 +163,18 @@ def create_information_schema_query(
sqlalchemy_engine: Engine | None = None,
) -> str:
"""Create query for getting table schemas from information schema."""
+ if (
+ Column is None
+ or MetaData is None
+ or Table is None
+ or and_ is None
+ or or_ is None
+ or union_all is None
+ ):
+ raise AirflowOptionalProviderFeatureException(
+ "sqlalchemy is required for SQL schema query generation. "
+ "Install it with: pip install 'apache-airflow-providers-openlineage[sqlalchemy]'"
+ )
Review Comment:
changed 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
Re: [PR] Make sqlalchemy optional dependency for openlineage provider [airflow]
Posted by "sunank200 (via GitHub)" <gi...@apache.org>.
sunank200 commented on code in PR #59921:
URL: https://github.com/apache/airflow/pull/59921#discussion_r2655388109
##########
providers/openlineage/pyproject.toml:
##########
@@ -66,6 +66,13 @@ dependencies = [
"openlineage-python>=1.41.0",
]
+# The optional dependencies should be modified in place in the generated file
+# Any change in the dependencies is preserved when the file is regenerated
+[project.optional-dependencies]
+"sqlalchemy" = [
+ "sqlalchemy[asyncio]>=1.4.49",
Review Comment:
Sure. Changed 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
Re: [PR] Make sqlalchemy optional dependency for openlineage provider [airflow]
Posted by "sunank200 (via GitHub)" <gi...@apache.org>.
sunank200 commented on code in PR #59921:
URL: https://github.com/apache/airflow/pull/59921#discussion_r2655407127
##########
providers/openlineage/src/airflow/providers/openlineage/sqlparser.py:
##########
@@ -497,6 +498,14 @@ def get_openlineage_facets_with_sql(
try:
sqlalchemy_engine = hook.get_sqlalchemy_engine()
+ except ImportError as e:
+ if "sqlalchemy" in str(e).lower() or "No module named 'sqlalchemy" in str(e):
+ raise AirflowOptionalProviderFeatureException(
Review Comment:
Sure. Changed 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