You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "mwojtyczka (via GitHub)" <gi...@apache.org> on 2026/03/13 12:57:21 UTC
[PR] Added missing aiofiles dependency [airflow]
mwojtyczka opened a new pull request, #63518:
URL: https://github.com/apache/airflow/pull/63518
<!-- SPDX-License-Identifier: Apache-2.0
https://www.apache.org/licenses/LICENSE-2.0 -->
* Adding missing dependency to the Databricks provider as per [issue](https://github.com/apache/airflow/discussions/63502)
<!--
Thank you for contributing!
Please provide above a brief description of the changes made in this pull request.
Write a good git commit message following this guide: http://chris.beams.io/posts/git-commit/
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 (in general) for the review if you do not see reaction for a few days
(72 Hours is the minimum reaction time you can expect from volunteers) - we sometimes miss notifications.
In case of an existing issue, reference it using one of the following:
* closes: #ISSUE
* related: #ISSUE
-->
---
##### Was generative AI tooling used to co-author this PR?
<!--
If generative AI tooling has been used in the process of authoring this PR, please
change below checkbox to `[X]` followed by the name of the tool, uncomment the "Generated-by".
-->
- No
<!--
Generated-by: [Tool Name] following [the guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
-->
---
* Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
* For fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
* When adding dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
* For significant user-facing changes create newsfragment: `{pr_number}.significant.rst`, in [airflow-core/newsfragments](https://github.com/apache/airflow/tree/main/airflow-core/newsfragments). You can add this file in a follow-up commit after the PR is created so you know the PR number.
--
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] Added missing aiofiles dependency [airflow]
Posted by "mwojtyczka (via GitHub)" <gi...@apache.org>.
mwojtyczka commented on code in PR #63518:
URL: https://github.com/apache/airflow/pull/63518#discussion_r2931259672
##########
providers/databricks/pyproject.toml:
##########
@@ -64,6 +64,7 @@ dependencies = [
"requests>=2.32.0,<3",
"databricks-sql-connector>=4.0.0",
"aiohttp>=3.9.2, <4",
+ "aiofiles>=23.2.0",
Review Comment:
For the standard “Airflow on Kubernetes” deployment that would work. However, I think this will not work if a custom image is used without cncf-kubernetes[aiofiles] extra
--
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] Added missing aiofiles dependency [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on code in PR #63518:
URL: https://github.com/apache/airflow/pull/63518#discussion_r2931283094
##########
providers/databricks/pyproject.toml:
##########
@@ -64,6 +64,7 @@ dependencies = [
"requests>=2.32.0,<3",
"databricks-sql-connector>=4.0.0",
"aiohttp>=3.9.2, <4",
+ "aiofiles>=23.2.0",
Review Comment:
See example of how we handle such cases:
https://github.com/apache/airflow/blob/702b51a7d4b8386e20a0646b299fbe618ca897d3/providers/standard/src/airflow/providers/standard/operators/python.py#L408-L417
We can't install everything on every provider. That would be madness.
Only things you actually need should be installed.
The last thing user should face is solving pip conflicts about packages he doesn't even need.
--
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] Fix broken import of `aiofiles` in `BaseDatabricksHook` [airflow]
Posted by "mwojtyczka (via GitHub)" <gi...@apache.org>.
mwojtyczka commented on code in PR #63518:
URL: https://github.com/apache/airflow/pull/63518#discussion_r2932557816
##########
providers/databricks/src/airflow/providers/databricks/hooks/databricks_base.py:
##########
@@ -599,8 +598,28 @@ def _get_k8s_projected_volume_token(self) -> str:
except PermissionError as e:
raise AirflowException(f"Permission denied reading token from {projected_token_path}") from e
+ @staticmethod
+ def _get_aiofiles():
+ """
+ Lazy-import aiofiles for async Kubernetes in-cluster authentication.
+
+ :return: The aiofiles module.
+ :raises AirflowOptionalProviderFeatureException: If aiofiles is not installed.
+ """
+ try:
+ import aiofiles
+
+ return aiofiles
Review Comment:
no worries, thanks for the review!
--
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] Fix broken import of `aiofiles` in `BaseDatabricksHook` [airflow]
Posted by "XD-DENG (via GitHub)" <gi...@apache.org>.
XD-DENG commented on code in PR #63518:
URL: https://github.com/apache/airflow/pull/63518#discussion_r2932598450
##########
providers/databricks/src/airflow/providers/databricks/hooks/databricks_base.py:
##########
@@ -599,8 +598,28 @@ def _get_k8s_projected_volume_token(self) -> str:
except PermissionError as e:
raise AirflowException(f"Permission denied reading token from {projected_token_path}") from e
+ @staticmethod
+ def _get_aiofiles():
+ """
+ Lazy-import aiofiles for async Kubernetes in-cluster authentication.
+
+ :return: The aiofiles module.
+ :raises AirflowOptionalProviderFeatureException: If aiofiles is not installed.
+ """
+ try:
+ import aiofiles
+
+ return aiofiles
+ except (ImportError, ModuleNotFoundError) as err:
Review Comment:
`ModuleNotFoundError` is a subclass of `ImportError`, so catching both is redundant. Should just be:
```python
except ImportError as err:
```
[Reference](https://docs.python.org/3/library/exceptions.html#ModuleNotFoundError)
--
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] Added missing aiofiles dependency [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on code in PR #63518:
URL: https://github.com/apache/airflow/pull/63518#discussion_r2931350586
##########
providers/databricks/pyproject.toml:
##########
@@ -64,6 +64,7 @@ dependencies = [
"requests>=2.32.0,<3",
"databricks-sql-connector>=4.0.0",
"aiohttp>=3.9.2, <4",
+ "aiofiles>=23.2.0",
Review Comment:
The `AirflowOptionalProviderFeatureException` was created specifically for such cases. so users will be aware that they are using an optional feature of a provider and missing something to make it work properly.
So my proposal is to move the import from top level to the function that actually use it and add a `AirflowOptionalProviderFeatureException` to notify users if depended provider is missing.
Let me know if that works
--
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] Added missing aiofiles dependency [airflow]
Posted by "mwojtyczka (via GitHub)" <gi...@apache.org>.
mwojtyczka commented on code in PR #63518:
URL: https://github.com/apache/airflow/pull/63518#discussion_r2931307913
##########
providers/databricks/pyproject.toml:
##########
@@ -64,6 +64,7 @@ dependencies = [
"requests>=2.32.0,<3",
"databricks-sql-connector>=4.0.0",
"aiohttp>=3.9.2, <4",
+ "aiofiles>=23.2.0",
Review Comment:
sounds good, if that is a generally used pattern in airflow we can use the same in databricks provider
--
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] Added missing aiofiles dependency [airflow]
Posted by "mwojtyczka (via GitHub)" <gi...@apache.org>.
mwojtyczka commented on code in PR #63518:
URL: https://github.com/apache/airflow/pull/63518#discussion_r2931259672
##########
providers/databricks/pyproject.toml:
##########
@@ -64,6 +64,7 @@ dependencies = [
"requests>=2.32.0,<3",
"databricks-sql-connector>=4.0.0",
"aiohttp>=3.9.2, <4",
+ "aiofiles>=23.2.0",
Review Comment:
For the standard “Airflow on Kubernetes” deployment that would work. However, this will not work if a custom image is used without aiofiles cncf-kubernetes extra while still using Databricks with K8s in-cluster auth
--
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] Added missing aiofiles dependency [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on code in PR #63518:
URL: https://github.com/apache/airflow/pull/63518#discussion_r2931211266
##########
providers/databricks/pyproject.toml:
##########
@@ -64,6 +64,7 @@ dependencies = [
"requests>=2.32.0,<3",
"databricks-sql-connector>=4.0.0",
"aiohttp>=3.9.2, <4",
+ "aiofiles>=23.2.0",
Review Comment:
I am not sure if this is the right fix(?)
aiofiles was added to support https://github.com/apache/airflow/pull/61458 which is only for k8s usage with databricks
I wonder if we should move the import of the package to the k8s related functions that way it will only be imported when used with K8s and the aiofiles is already a direct dependency of the k8s provider
https://github.com/apache/airflow/blob/92ffe9e6c48b0d07b80321c6e2bca3c7be539225/providers/cncf/kubernetes/pyproject.toml#L61
--
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] Fix broken import of `aiofiles` in `BaseDatabricksHook` [airflow]
Posted by "mwojtyczka (via GitHub)" <gi...@apache.org>.
mwojtyczka commented on code in PR #63518:
URL: https://github.com/apache/airflow/pull/63518#discussion_r2932393724
##########
providers/databricks/src/airflow/providers/databricks/hooks/databricks_base.py:
##########
@@ -599,8 +598,28 @@ def _get_k8s_projected_volume_token(self) -> str:
except PermissionError as e:
raise AirflowException(f"Permission denied reading token from {projected_token_path}") from e
+ @staticmethod
+ def _get_aiofiles():
+ """
+ Lazy-import aiofiles for async Kubernetes in-cluster authentication.
+
+ :return: The aiofiles module.
+ :raises AirflowOptionalProviderFeatureException: If aiofiles is not installed.
+ """
+ try:
+ import aiofiles
+
+ return aiofiles
Review Comment:
can you be more specific?
--
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] Added missing aiofiles dependency [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on code in PR #63518:
URL: https://github.com/apache/airflow/pull/63518#discussion_r2931283094
##########
providers/databricks/pyproject.toml:
##########
@@ -64,6 +64,7 @@ dependencies = [
"requests>=2.32.0,<3",
"databricks-sql-connector>=4.0.0",
"aiohttp>=3.9.2, <4",
+ "aiofiles>=23.2.0",
Review Comment:
See example of how we handle such cases:
https://github.com/apache/airflow/blob/702b51a7d4b8386e20a0646b299fbe618ca897d3/providers/standard/src/airflow/providers/standard/operators/python.py#L408-L417
https://github.com/apache/airflow/blob/4d3c5ead3b570f9c6a395d7aa63d856270fc5619/providers/apache/impala/src/airflow/providers/apache/impala/hooks/impala.py#L54-L60
https://github.com/apache/airflow/blob/4d3c5ead3b570f9c6a395d7aa63d856270fc5619/providers/vertica/src/airflow/providers/vertica/hooks/vertica.py#L143-L149
We can't install everything on every provider. That would be madness.
Only things you actually need should be installed.
The last thing user should face is solving pip conflicts about packages he doesn't even need.
--
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] Added missing aiofiles dependency [airflow]
Posted by "mwojtyczka (via GitHub)" <gi...@apache.org>.
mwojtyczka commented on code in PR #63518:
URL: https://github.com/apache/airflow/pull/63518#discussion_r2931259672
##########
providers/databricks/pyproject.toml:
##########
@@ -64,6 +64,7 @@ dependencies = [
"requests>=2.32.0,<3",
"databricks-sql-connector>=4.0.0",
"aiohttp>=3.9.2, <4",
+ "aiofiles>=23.2.0",
Review Comment:
For the standard “Airflow on Kubernetes” deployment that would work. However, I think this will not work if a custom image is used without cncf-kubernetes. Do we want to have this transient dependency on cncf-kubernetes? if that is ok, we can import lazily
--
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] Added missing aiofiles dependency [airflow]
Posted by "mwojtyczka (via GitHub)" <gi...@apache.org>.
mwojtyczka commented on code in PR #63518:
URL: https://github.com/apache/airflow/pull/63518#discussion_r2931259672
##########
providers/databricks/pyproject.toml:
##########
@@ -64,6 +64,7 @@ dependencies = [
"requests>=2.32.0,<3",
"databricks-sql-connector>=4.0.0",
"aiohttp>=3.9.2, <4",
+ "aiofiles>=23.2.0",
Review Comment:
For the standard “Airflow on Kubernetes” deployment that would work. However, this will not work if a custom image is used without aiofiles cncf-kubernetes extra
##########
providers/databricks/pyproject.toml:
##########
@@ -64,6 +64,7 @@ dependencies = [
"requests>=2.32.0,<3",
"databricks-sql-connector>=4.0.0",
"aiohttp>=3.9.2, <4",
+ "aiofiles>=23.2.0",
Review Comment:
For the standard “Airflow on Kubernetes” deployment that would work. However, I think this will not work if a custom image is used without aiofiles cncf-kubernetes extra
--
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] Added missing aiofiles dependency [airflow]
Posted by "mwojtyczka (via GitHub)" <gi...@apache.org>.
mwojtyczka commented on code in PR #63518:
URL: https://github.com/apache/airflow/pull/63518#discussion_r2931307913
##########
providers/databricks/pyproject.toml:
##########
@@ -64,6 +64,7 @@ dependencies = [
"requests>=2.32.0,<3",
"databricks-sql-connector>=4.0.0",
"aiohttp>=3.9.2, <4",
+ "aiofiles>=23.2.0",
Review Comment:
sounds good, if that is a generally used pattern in airflow, and ariflow on kubernetes installation always depends on cncf-kubernetes we can use the same in databricks provider
--
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] Fix broken import of `aiofiles` in `BaseDatabricksHook` [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on PR #63518:
URL: https://github.com/apache/airflow/pull/63518#issuecomment-4133946481
> Fresh install: I still faced the same aiofile missing issue on airflow 3.1.8
This is not related to Airflow version but to databricks provider.
The release is in progress. You can test with
apache-airflow-providers-databricks==7.12.0rc1
--
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] Added missing aiofiles dependency [airflow]
Posted by "mwojtyczka (via GitHub)" <gi...@apache.org>.
mwojtyczka commented on code in PR #63518:
URL: https://github.com/apache/airflow/pull/63518#discussion_r2931259672
##########
providers/databricks/pyproject.toml:
##########
@@ -64,6 +64,7 @@ dependencies = [
"requests>=2.32.0,<3",
"databricks-sql-connector>=4.0.0",
"aiohttp>=3.9.2, <4",
+ "aiofiles>=23.2.0",
Review Comment:
For the standard “Airflow on Kubernetes” deployment that would work. However, I think this will not work if a custom image is used without cncf-kubernetes[aiofiles] extra.I understand that Airflow on Kubernetes is typically run from the official image or an image built with the default ariflow extras that includes the cncf-kubernetes. But I'm not sure this is always the case? if we believe there is no risk, then we can use lazy import instead
--
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] Added missing aiofiles dependency [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on code in PR #63518:
URL: https://github.com/apache/airflow/pull/63518#discussion_r2931283094
##########
providers/databricks/pyproject.toml:
##########
@@ -64,6 +64,7 @@ dependencies = [
"requests>=2.32.0,<3",
"databricks-sql-connector>=4.0.0",
"aiohttp>=3.9.2, <4",
+ "aiofiles>=23.2.0",
Review Comment:
See example of how we handle such cases:
https://github.com/apache/airflow/blob/86d5b27f92207571ebe0c29a42c42abbf6f8cb8c/airflow-core/src/airflow/example_dags/example_kubernetes_executor.py#L35-L42
We can't install everything on every provider. That would be madness.
Only things you actually need should be installed.
The last thing user should face is solving pip conflicts about packages he doesn't even need.
--
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] Added missing aiofiles dependency [airflow]
Posted by "mwojtyczka (via GitHub)" <gi...@apache.org>.
mwojtyczka commented on code in PR #63518:
URL: https://github.com/apache/airflow/pull/63518#discussion_r2931259672
##########
providers/databricks/pyproject.toml:
##########
@@ -64,6 +64,7 @@ dependencies = [
"requests>=2.32.0,<3",
"databricks-sql-connector>=4.0.0",
"aiohttp>=3.9.2, <4",
+ "aiofiles>=23.2.0",
Review Comment:
For the standard “Airflow on Kubernetes” deployment that would work. However, I think this will not work if a custom image is used without cncf-kubernetes. Do we want to have this transient on cncf-kubernetes?
--
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] Fix broken import of `aiofiles` in `BaseDatabricksHook` [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on code in PR #63518:
URL: https://github.com/apache/airflow/pull/63518#discussion_r2932450516
##########
providers/databricks/src/airflow/providers/databricks/hooks/databricks_base.py:
##########
@@ -599,8 +598,28 @@ def _get_k8s_projected_volume_token(self) -> str:
except PermissionError as e:
raise AirflowException(f"Permission denied reading token from {projected_token_path}") from e
+ @staticmethod
+ def _get_aiofiles():
+ """
+ Lazy-import aiofiles for async Kubernetes in-cluster authentication.
+
+ :return: The aiofiles module.
+ :raises AirflowOptionalProviderFeatureException: If aiofiles is not installed.
+ """
+ try:
+ import aiofiles
+
+ return aiofiles
Review Comment:
Ah sorry you implemented as a function. Didn't see it intially. I reviewed form the mobile browser
--
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] Fix broken import of `aiofiles` in `BaseDatabricksHook` [airflow]
Posted by "mwojtyczka (via GitHub)" <gi...@apache.org>.
mwojtyczka commented on code in PR #63518:
URL: https://github.com/apache/airflow/pull/63518#discussion_r2932393724
##########
providers/databricks/src/airflow/providers/databricks/hooks/databricks_base.py:
##########
@@ -599,8 +598,28 @@ def _get_k8s_projected_volume_token(self) -> str:
except PermissionError as e:
raise AirflowException(f"Permission denied reading token from {projected_token_path}") from e
+ @staticmethod
+ def _get_aiofiles():
+ """
+ Lazy-import aiofiles for async Kubernetes in-cluster authentication.
+
+ :return: The aiofiles module.
+ :raises AirflowOptionalProviderFeatureException: If aiofiles is not installed.
+ """
+ try:
+ import aiofiles
+
+ return aiofiles
Review Comment:
can you be more specific pls?
--
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] Added missing aiofiles dependency [airflow]
Posted by "mwojtyczka (via GitHub)" <gi...@apache.org>.
mwojtyczka commented on code in PR #63518:
URL: https://github.com/apache/airflow/pull/63518#discussion_r2931259672
##########
providers/databricks/pyproject.toml:
##########
@@ -64,6 +64,7 @@ dependencies = [
"requests>=2.32.0,<3",
"databricks-sql-connector>=4.0.0",
"aiohttp>=3.9.2, <4",
+ "aiofiles>=23.2.0",
Review Comment:
For the standard “Airflow on Kubernetes” deployment that would work. However, I think this will not work if a custom image is used without cncf-kubernetes[aiofiles] extra.
I understand that Airflow on Kubernetes is typically run from the official image or an image built with the default ariflow extras that includes the cncf-kubernetes. But I'm not sure this is always the case?
--
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] Added missing aiofiles dependency [airflow]
Posted by "mwojtyczka (via GitHub)" <gi...@apache.org>.
mwojtyczka commented on code in PR #63518:
URL: https://github.com/apache/airflow/pull/63518#discussion_r2931307913
##########
providers/databricks/pyproject.toml:
##########
@@ -64,6 +64,7 @@ dependencies = [
"requests>=2.32.0,<3",
"databricks-sql-connector>=4.0.0",
"aiohttp>=3.9.2, <4",
+ "aiofiles>=23.2.0",
Review Comment:
sounds good, if that is a generally used pattern in airflow, and ariflow on kubernetes installation always depends on cncf-kubernetes we can use this approach 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] Added missing aiofiles dependency [airflow]
Posted by "mwojtyczka (via GitHub)" <gi...@apache.org>.
mwojtyczka commented on code in PR #63518:
URL: https://github.com/apache/airflow/pull/63518#discussion_r2931307913
##########
providers/databricks/pyproject.toml:
##########
@@ -64,6 +64,7 @@ dependencies = [
"requests>=2.32.0,<3",
"databricks-sql-connector>=4.0.0",
"aiohttp>=3.9.2, <4",
+ "aiofiles>=23.2.0",
Review Comment:
sounds good, if that is a generally used pattern in airflow and kubernetes installation always depends on cncf-kubernetes we can use the same in databricks provider
--
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] Fix broken import of `aiofiles` in `BaseDatabricksHook` [airflow]
Posted by "ayushpattnaik (via GitHub)" <gi...@apache.org>.
ayushpattnaik commented on PR #63518:
URL: https://github.com/apache/airflow/pull/63518#issuecomment-4133840222
I still faced the same aiofile missing 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] Added missing aiofiles dependency [airflow]
Posted by "mwojtyczka (via GitHub)" <gi...@apache.org>.
mwojtyczka commented on code in PR #63518:
URL: https://github.com/apache/airflow/pull/63518#discussion_r2931259672
##########
providers/databricks/pyproject.toml:
##########
@@ -64,6 +64,7 @@ dependencies = [
"requests>=2.32.0,<3",
"databricks-sql-connector>=4.0.0",
"aiohttp>=3.9.2, <4",
+ "aiofiles>=23.2.0",
Review Comment:
For the standard “Airflow on Kubernetes” deployment that would work. However, I think this will not work if a custom image is used without cncf-kubernetes. Do we want to have this transient dependency on cncf-kubernetes?
--
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] Added missing aiofiles dependency [airflow]
Posted by "mwojtyczka (via GitHub)" <gi...@apache.org>.
mwojtyczka commented on code in PR #63518:
URL: https://github.com/apache/airflow/pull/63518#discussion_r2932070012
##########
providers/databricks/pyproject.toml:
##########
@@ -64,6 +64,7 @@ dependencies = [
"requests>=2.32.0,<3",
"databricks-sql-connector>=4.0.0",
"aiohttp>=3.9.2, <4",
+ "aiofiles>=23.2.0",
Review Comment:
Updated as per our discussion
--
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] Added missing aiofiles dependency [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on code in PR #63518:
URL: https://github.com/apache/airflow/pull/63518#discussion_r2932345337
##########
providers/databricks/src/airflow/providers/databricks/hooks/databricks_base.py:
##########
@@ -599,8 +598,28 @@ def _get_k8s_projected_volume_token(self) -> str:
except PermissionError as e:
raise AirflowException(f"Permission denied reading token from {projected_token_path}") from e
+ @staticmethod
+ def _get_aiofiles():
+ """
+ Lazy-import aiofiles for async Kubernetes in-cluster authentication.
+
+ :return: The aiofiles module.
+ :raises AirflowOptionalProviderFeatureException: If aiofiles is not installed.
+ """
+ try:
+ import aiofiles
+
+ return aiofiles
Review Comment:
I assume this is a wrong copy/paste? :)
--
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] Fix broken import of `aiofiles` in `BaseDatabricksHook` [airflow]
Posted by "mwojtyczka (via GitHub)" <gi...@apache.org>.
mwojtyczka commented on code in PR #63518:
URL: https://github.com/apache/airflow/pull/63518#discussion_r2934075760
##########
providers/databricks/src/airflow/providers/databricks/hooks/databricks_base.py:
##########
@@ -599,8 +598,28 @@ def _get_k8s_projected_volume_token(self) -> str:
except PermissionError as e:
raise AirflowException(f"Permission denied reading token from {projected_token_path}") from e
+ @staticmethod
+ def _get_aiofiles():
+ """
+ Lazy-import aiofiles for async Kubernetes in-cluster authentication.
+
+ :return: The aiofiles module.
+ :raises AirflowOptionalProviderFeatureException: If aiofiles is not installed.
+ """
+ try:
+ import aiofiles
+
+ return aiofiles
+ except (ImportError, ModuleNotFoundError) as err:
Review Comment:
fixed
--
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] Fix broken import of `aiofiles` in `BaseDatabricksHook` [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal merged PR #63518:
URL: https://github.com/apache/airflow/pull/63518
--
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