hussein-awala commented on code in PR #33786:
URL: https://github.com/apache/airflow/pull/33786#discussion_r1320302570
##########
airflow/providers/amazon/aws/operators/ecs.py:
##########
@@ -496,15 +496,12 @@ def __init__(
self.tags = tags
self.awslogs_group = awslogs_group
self.awslogs_stream_prefix = awslogs_stream_prefix
- self.awslogs_region = awslogs_region
+ self.awslogs_region = awslogs_region or self.region
self.awslogs_fetch_interval = awslogs_fetch_interval
self.propagate_tags = propagate_tags
self.reattach = reattach
self.number_logs_exception = number_logs_exception
- if self.awslogs_region is None:
- self.awslogs_region = self.region
Review Comment:
I wonder why the condition was `is None`, should empty string be accepted?
##########
airflow/providers/amazon/aws/operators/sagemaker.py:
##########
@@ -666,6 +660,12 @@ def expand_role(self) -> None:
config["ExecutionRoleArn"] =
hook.expand_role(config["ExecutionRoleArn"])
def execute(self, context: Context) -> dict:
+ if self.action_if_job_exists not in ("increment", "fail", "timestamp"):
+ raise AirflowException(
+ f"Argument action_if_job_exists accepts only 'timestamp',
'increment' and 'fail'. \
+ Provided value: '{self.action_if_job_exists}'."
+ )
+
Review Comment:
With this change, instead of warning the user in dev time to fix the param,
you move it to runtime. Personally I prefer to keep it in init :thinking:
##########
airflow/providers/amazon/aws/operators/sagemaker.py:
##########
@@ -1026,6 +1021,11 @@ def _create_integer_fields(self) -> None:
]
def execute(self, context: Context) -> dict:
+ if self.action_if_job_exists not in {"timestamp", "increment", "fail"}:
+ raise AirflowException(
+ f"Argument action_if_job_exists accepts only 'timestamp',
'increment' and 'fail'. \
+ Provided value: '{self.action_if_job_exists}'."
+ )
Review Comment:
same 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
