You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/08/19 18:35:48 UTC
[GitHub] [airflow] vincbeck opened a new pull request, #25839: Fix RDS system test
vincbeck opened a new pull request, #25839:
URL: https://github.com/apache/airflow/pull/25839
The status of the snapshot can be `creating` and `copying`. I ran into that issue while running the system test.
[See doc](https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/rds.html#RDS.Client.create_db_cluster_snapshot)
--
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
[GitHub] [airflow] vincbeck commented on a diff in pull request #25839: Fix RDS system test
Posted by GitBox <gi...@apache.org>.
vincbeck commented on code in PR #25839:
URL: https://github.com/apache/airflow/pull/25839#discussion_r951529093
##########
airflow/providers/amazon/aws/operators/rds.py:
##########
@@ -278,7 +278,7 @@ def execute(self, context: 'Context') -> str:
self._await_status(
item_type,
self.target_db_snapshot_identifier,
- wait_statuses=['copying'],
+ wait_statuses=['creating', 'copying'],
ok_statuses=['available'],
Review Comment:
We want to fail fast in case of error status. If the status is `error` or any terminal state which reflect an error, why waiting?
--
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
[GitHub] [airflow] eladkal commented on a diff in pull request #25839: Fix RDS system test
Posted by GitBox <gi...@apache.org>.
eladkal commented on code in PR #25839:
URL: https://github.com/apache/airflow/pull/25839#discussion_r950801993
##########
airflow/providers/amazon/aws/operators/rds.py:
##########
@@ -278,7 +278,7 @@ def execute(self, context: 'Context') -> str:
self._await_status(
item_type,
self.target_db_snapshot_identifier,
- wait_statuses=['copying'],
+ wait_statuses=['creating', 'copying'],
ok_statuses=['available'],
Review Comment:
I'm approving to unblock the system test work but this seems a bit strange logic?
If the goal is to wait for the ok status then why we care about the wait status?
Shouldn't we just wait till the status is `available`? meaning that anything that isn't the ok_status we just continue to wait.
What am I missing 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
[GitHub] [airflow] eladkal commented on a diff in pull request #25839: Fix RDS system test
Posted by GitBox <gi...@apache.org>.
eladkal commented on code in PR #25839:
URL: https://github.com/apache/airflow/pull/25839#discussion_r951539035
##########
airflow/providers/amazon/aws/operators/rds.py:
##########
@@ -278,7 +278,7 @@ def execute(self, context: 'Context') -> str:
self._await_status(
item_type,
self.target_db_snapshot_identifier,
- wait_statuses=['copying'],
+ wait_statuses=['creating', 'copying'],
ok_statuses=['available'],
Review Comment:
This is an operator that hat the option to wait till the action is completed.
To know if it's completed there is a target status (listed as `ok_statuses`)
The need to specify also what are the wait statuses for this scenario seems unnecessary to me.
--
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
[GitHub] [airflow] eladkal merged pull request #25839: Fix RDS system test
Posted by GitBox <gi...@apache.org>.
eladkal merged PR #25839:
URL: https://github.com/apache/airflow/pull/25839
--
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