Re: [PR] Update EKS Operators and Sensors to inherit AWS base classes [airflow]
eladkal merged PR #48192: URL: https://github.com/apache/airflow/pull/48192 -- 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] Update EKS Operators and Sensors to inherit AWS base classes [airflow]
ellisms commented on code in PR #48192:
URL: https://github.com/apache/airflow/pull/48192#discussion_r2036144832
##
providers/amazon/src/airflow/providers/amazon/aws/operators/eks.py:
##
@@ -253,20 +253,23 @@ def __init__(
self.wait_for_completion = wait_for_completion
self.waiter_delay = waiter_delay
self.waiter_max_attempts = waiter_max_attempts
-self.aws_conn_id = aws_conn_id
-self.region = region
self.nodegroup_name = nodegroup_name
self.create_nodegroup_kwargs = create_nodegroup_kwargs or {}
self.fargate_selectors = fargate_selectors or [{"namespace":
DEFAULT_NAMESPACE_NAME}]
self.fargate_profile_name = fargate_profile_name
self.deferrable = deferrable
+self.region = region
Review Comment:
I ran into failed template field tests if the class's attributes didn't
match the template fields. Since we are keeping `region` in the template fields
for now, I left the attribute.
--
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] Update EKS Operators and Sensors to inherit AWS base classes [airflow]
vincbeck commented on PR #48192: URL: https://github.com/apache/airflow/pull/48192#issuecomment-2791008235 (tests are failing) -- 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] Update EKS Operators and Sensors to inherit AWS base classes [airflow]
vincbeck commented on code in PR #48192:
URL: https://github.com/apache/airflow/pull/48192#discussion_r2036148940
##
providers/amazon/src/airflow/providers/amazon/aws/operators/eks.py:
##
@@ -253,20 +253,23 @@ def __init__(
self.wait_for_completion = wait_for_completion
self.waiter_delay = waiter_delay
self.waiter_max_attempts = waiter_max_attempts
-self.aws_conn_id = aws_conn_id
-self.region = region
self.nodegroup_name = nodegroup_name
self.create_nodegroup_kwargs = create_nodegroup_kwargs or {}
self.fargate_selectors = fargate_selectors or [{"namespace":
DEFAULT_NAMESPACE_NAME}]
self.fargate_profile_name = fargate_profile_name
self.deferrable = deferrable
+self.region = region
Review Comment:
Sounds 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-unsubscr...@airflow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Update EKS Operators and Sensors to inherit AWS base classes [airflow]
vincbeck commented on code in PR #48192:
URL: https://github.com/apache/airflow/pull/48192#discussion_r2036130881
##
providers/amazon/src/airflow/providers/amazon/aws/operators/eks.py:
##
@@ -253,20 +253,23 @@ def __init__(
self.wait_for_completion = wait_for_completion
self.waiter_delay = waiter_delay
self.waiter_max_attempts = waiter_max_attempts
-self.aws_conn_id = aws_conn_id
-self.region = region
self.nodegroup_name = nodegroup_name
self.create_nodegroup_kwargs = create_nodegroup_kwargs or {}
self.fargate_selectors = fargate_selectors or [{"namespace":
DEFAULT_NAMESPACE_NAME}]
self.fargate_profile_name = fargate_profile_name
self.deferrable = deferrable
+self.region = region
Review Comment:
Do we need 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-unsubscr...@airflow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Update EKS Operators and Sensors to inherit AWS base classes [airflow]
ellisms commented on code in PR #48192:
URL: https://github.com/apache/airflow/pull/48192#discussion_r2033712358
##
providers/amazon/src/airflow/providers/amazon/aws/operators/eks.py:
##
@@ -253,20 +253,22 @@ def __init__(
self.wait_for_completion = wait_for_completion
self.waiter_delay = waiter_delay
self.waiter_max_attempts = waiter_max_attempts
-self.aws_conn_id = aws_conn_id
-self.region = region
self.nodegroup_name = nodegroup_name
self.create_nodegroup_kwargs = create_nodegroup_kwargs or {}
self.fargate_selectors = fargate_selectors or [{"namespace":
DEFAULT_NAMESPACE_NAME}]
self.fargate_profile_name = fargate_profile_name
self.deferrable = deferrable
+self.region = region
super().__init__(
**kwargs,
)
-@cached_property
-def hook(self) -> EksHook:
-return EksHook(aws_conn_id=self.aws_conn_id, region_name=self.region)
+if region is not None:
+warnings.warn(
+message="Parameter `region` will be deprecated. Use the
parameter `region_name` instead",
+category=AirflowProviderDeprecationWarning,
+stacklevel=2,
+)
Review Comment:
Good catch. I'll update 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-unsubscr...@airflow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Update EKS Operators and Sensors to inherit AWS base classes [airflow]
vincbeck commented on code in PR #48192:
URL: https://github.com/apache/airflow/pull/48192#discussion_r2033111460
##
providers/amazon/src/airflow/providers/amazon/aws/operators/eks.py:
##
@@ -253,20 +253,22 @@ def __init__(
self.wait_for_completion = wait_for_completion
self.waiter_delay = waiter_delay
self.waiter_max_attempts = waiter_max_attempts
-self.aws_conn_id = aws_conn_id
-self.region = region
self.nodegroup_name = nodegroup_name
self.create_nodegroup_kwargs = create_nodegroup_kwargs or {}
self.fargate_selectors = fargate_selectors or [{"namespace":
DEFAULT_NAMESPACE_NAME}]
self.fargate_profile_name = fargate_profile_name
self.deferrable = deferrable
+self.region = region
super().__init__(
**kwargs,
)
-@cached_property
-def hook(self) -> EksHook:
-return EksHook(aws_conn_id=self.aws_conn_id, region_name=self.region)
+if region is not None:
+warnings.warn(
+message="Parameter `region` will be deprecated. Use the
parameter `region_name` instead",
+category=AirflowProviderDeprecationWarning,
+stacklevel=2,
+)
Review Comment:
Should we set `self.region = region_name` in that 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-unsubscr...@airflow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Update EKS Operators and Sensors to inherit AWS base classes [airflow]
ellisms commented on code in PR #48192: URL: https://github.com/apache/airflow/pull/48192#discussion_r2019775456 ## providers/amazon/src/airflow/providers/amazon/aws/operators/eks.py: ## @@ -214,8 +215,6 @@ class EksCreateClusterOperator(BaseOperator): "fargate_selectors", "create_fargate_profile_kwargs", "wait_for_completion", -"aws_conn_id", -"region", Review Comment: I added back region and also added a deprecation warning. -- 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] Update EKS Operators and Sensors to inherit AWS base classes [airflow]
o-nikolas commented on code in PR #48192: URL: https://github.com/apache/airflow/pull/48192#discussion_r2010532445 ## providers/amazon/src/airflow/providers/amazon/aws/operators/eks.py: ## @@ -214,8 +215,6 @@ class EksCreateClusterOperator(BaseOperator): "fargate_selectors", "create_fargate_profile_kwargs", "wait_for_completion", -"aws_conn_id", -"region", Review Comment: This is a breaking change, we should accept both for a time while logging deprecation warnings, and then we can eventually remove it in a major release of the provider package. -- 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
