Re: [PR] Fix aws_conn_id defaulting after dag.test was updated to use TaskSDK. [airflow]
vincbeck merged PR #50515: URL: https://github.com/apache/airflow/pull/50515 -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix aws_conn_id defaulting after dag.test was updated to use TaskSDK. [airflow]
ramitkataria commented on code in PR #50515: URL: https://github.com/apache/airflow/pull/50515#discussion_r2101370215 ## providers/amazon/src/airflow/providers/amazon/aws/hooks/base_aws.py: ## @@ -603,12 +607,22 @@ def conn_config(self) -> AwsConnectionWrapper: """Get the Airflow Connection object and wrap it in helper (cached).""" connection = None if self.aws_conn_id: +possible_exceptions = ( +(AirflowNotFoundException, AirflowRuntimeError) Review Comment: Yea I think we can resolve and merge this for now and discuss on slack instead? If we end up going with the approach I proposed, then the changes in this PR wouldn't be needed anymore and instead, the change would be in sdk 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix aws_conn_id defaulting after dag.test was updated to use TaskSDK. [airflow]
ferruzzi commented on code in PR #50515: URL: https://github.com/apache/airflow/pull/50515#discussion_r2100991428 ## providers/amazon/src/airflow/providers/amazon/aws/hooks/base_aws.py: ## @@ -603,12 +607,22 @@ def conn_config(self) -> AwsConnectionWrapper: """Get the Airflow Connection object and wrap it in helper (cached).""" connection = None if self.aws_conn_id: +possible_exceptions = ( +(AirflowNotFoundException, AirflowRuntimeError) Review Comment: I think it's a valid discussion, but we shouldn't be blocking this PR for it.Unless I am missing something and there is something else I need to implement 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix aws_conn_id defaulting after dag.test was updated to use TaskSDK. [airflow]
vincbeck commented on code in PR #50515: URL: https://github.com/apache/airflow/pull/50515#discussion_r2100799061 ## providers/amazon/src/airflow/providers/amazon/aws/hooks/base_aws.py: ## @@ -603,12 +607,22 @@ def conn_config(self) -> AwsConnectionWrapper: """Get the Airflow Connection object and wrap it in helper (cached).""" connection = None if self.aws_conn_id: +possible_exceptions = ( +(AirflowNotFoundException, AirflowRuntimeError) Review Comment: Can we resolve this conversation? -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix aws_conn_id defaulting after dag.test was updated to use TaskSDK. [airflow]
ramitkataria commented on PR #50515: URL: https://github.com/apache/airflow/pull/50515#issuecomment-2877535191 I'm curious how you're still getting the `AirflowRuntimeError` even after the change in #47648 -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix aws_conn_id defaulting after dag.test was updated to use TaskSDK. [airflow]
o-nikolas commented on code in PR #50515:
URL: https://github.com/apache/airflow/pull/50515#discussion_r2087399033
##
providers/amazon/src/airflow/providers/amazon/aws/hooks/base_aws.py:
##
@@ -58,12 +58,16 @@
from airflow.providers.amazon.aws.utils.connection_wrapper import
AwsConnectionWrapper
from airflow.providers.amazon.aws.utils.identifiers import generate_uuid
from airflow.providers.amazon.aws.utils.suppress import return_on_error
+from airflow.providers.common.compat.version_compat import AIRFLOW_V_3_0_PLUS
from airflow.providers_manager import ProvidersManager
from airflow.utils.helpers import exactly_one
from airflow.utils.log.logging_mixin import LoggingMixin
BaseAwsConnection = TypeVar("BaseAwsConnection", bound=Union[boto3.client,
boto3.resource])
+if AIRFLOW_V_3_0_PLUS:
+from airflow.sdk.exceptions import AirflowRuntimeError
Review Comment:
Yeah another branch is needed below to optionally use this Exception.
--
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-unsubscr...@airflow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Fix aws_conn_id defaulting after dag.test was updated to use TaskSDK. [airflow]
ferruzzi commented on code in PR #50515: URL: https://github.com/apache/airflow/pull/50515#discussion_r2087283953 ## providers/amazon/src/airflow/providers/amazon/aws/hooks/base_aws.py: ## @@ -59,6 +59,7 @@ from airflow.providers.amazon.aws.utils.identifiers import generate_uuid from airflow.providers.amazon.aws.utils.suppress import return_on_error from airflow.providers_manager import ProvidersManager +from airflow.sdk.exceptions import AirflowRuntimeError Review Comment: Had to rebase, fixed this in [3779cee](https://github.com/apache/airflow/pull/50515/commits/3779cee61c627831d9672f239fa33eb6d641844d). 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix aws_conn_id defaulting after dag.test was updated to use TaskSDK. [airflow]
eladkal commented on code in PR #50515: URL: https://github.com/apache/airflow/pull/50515#discussion_r2086030500 ## providers/amazon/src/airflow/providers/amazon/aws/hooks/base_aws.py: ## @@ -59,6 +59,7 @@ from airflow.providers.amazon.aws.utils.identifiers import generate_uuid from airflow.providers.amazon.aws.utils.suppress import return_on_error from airflow.providers_manager import ProvidersManager +from airflow.sdk.exceptions import AirflowRuntimeError Review Comment: This needs to be under version check as needs to be compatible with Airflow 2.10.0 and sdk path did not exist in 2.10 -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
