vincbeck commented on code in PR #39743:
URL: https://github.com/apache/airflow/pull/39743#discussion_r1610065344
##########
airflow/providers/amazon/aws/operators/emr.py:
##########
@@ -742,10 +742,21 @@ def __init__(
waiter_max_attempts: int | None = None,
waiter_delay: int | None = None,
waiter_countdown: int | None = None,
- waiter_check_interval_seconds: int = 60,
+ waiter_check_interval_seconds: int,
Review Comment:
This is a breaking change, we should no do that.
```suggestion
waiter_check_interval_seconds: int | None = None,
```
##########
airflow/providers/amazon/aws/operators/emr.py:
##########
@@ -742,10 +742,21 @@ def __init__(
waiter_max_attempts: int | None = None,
waiter_delay: int | None = None,
waiter_countdown: int | None = None,
- waiter_check_interval_seconds: int = 60,
+ waiter_check_interval_seconds: int,
deferrable: bool = conf.getboolean("operators", "default_deferrable",
fallback=False),
**kwargs: Any,
):
+ if waiter_check_interval_seconds:
+ warnings.warn(
+ "The parameter waiter_check_interval_seconds has been
deprecated to "
+ "standardize naming conventions. Please use waiter_delay
instead. In the "
Review Comment:
```suggestion
"standardize naming conventions. Please `use waiter_delay
instead`. In the "
```
##########
airflow/providers/amazon/aws/operators/emr.py:
##########
@@ -742,10 +742,21 @@ def __init__(
waiter_max_attempts: int | None = None,
waiter_delay: int | None = None,
waiter_countdown: int | None = None,
- waiter_check_interval_seconds: int = 60,
+ waiter_check_interval_seconds: int,
deferrable: bool = conf.getboolean("operators", "default_deferrable",
fallback=False),
**kwargs: Any,
):
+ if waiter_check_interval_seconds:
+ warnings.warn(
+ "The parameter waiter_check_interval_seconds has been
deprecated to "
Review Comment:
```suggestion
"The parameter `waiter_check_interval_seconds` has been
deprecated to "
```
##########
airflow/providers/amazon/aws/operators/emr.py:
##########
@@ -742,10 +742,21 @@ def __init__(
waiter_max_attempts: int | None = None,
waiter_delay: int | None = None,
waiter_countdown: int | None = None,
- waiter_check_interval_seconds: int = 60,
+ waiter_check_interval_seconds: int,
deferrable: bool = conf.getboolean("operators", "default_deferrable",
fallback=False),
**kwargs: Any,
):
+ if waiter_check_interval_seconds:
+ warnings.warn(
+ "The parameter waiter_check_interval_seconds has been
deprecated to "
+ "standardize naming conventions. Please use waiter_delay
instead. In the "
+ "future this will default to None and defer to the waiter's
default value.",
+ AirflowProviderDeprecationWarning,
+ stacklevel=2,
+ )
+ waiter_delay = waiter_check_interval_seconds
Review Comment:
I dont think you should do that, you might override `waiter_delay` set by
the user.
```suggestion
```
##########
airflow/providers/amazon/aws/operators/emr.py:
##########
@@ -757,23 +768,15 @@ def __init__(
# waiter_countdown defaults to never timing out, which is not
supported
# by boto waiters, so we will set it here to "a very long time"
for now.
waiter_max_attempts = (waiter_countdown or 999) //
waiter_check_interval_seconds
- if waiter_check_interval_seconds:
- warnings.warn(
- "The parameter waiter_check_interval_seconds has been
deprecated to "
- "standardize naming conventions. Please use waiter_delay
instead. In the "
- "future this will default to None and defer to the waiter's
default value.",
- AirflowProviderDeprecationWarning,
- stacklevel=2,
- )
- waiter_delay = waiter_check_interval_seconds
+
super().__init__(**kwargs)
self.aws_conn_id = aws_conn_id
self.emr_conn_id = emr_conn_id
self.job_flow_overrides = job_flow_overrides or {}
self.region_name = region_name
self.wait_for_completion = wait_for_completion
self.waiter_max_attempts = waiter_max_attempts or 60
- self.waiter_delay = waiter_delay or 30
+ self.waiter_delay = waiter_delay or 60
Review Comment:
```suggestion
self.waiter_delay = waiter_delay or waiter_check_interval_seconds or
60
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
