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/07/20 21:49:49 UTC

[GitHub] [airflow] ferruzzi opened a new pull request, #25205: Convert RDS Export Sample DAG to System Test (AIP-47)

ferruzzi opened a new pull request, #25205:
URL: https://github.com/apache/airflow/pull/25205

   Follows the template set forward in https://github.com/apache/airflow/pull/24643
   
   - Also fixes a mistake in the docstring for `_await_status` which incorrect described the method's behavior.
   - Also adds a "wait_for_completion" flag to the relevant operators as seen in many other operators to allow for easier unit testing of the operators and unit testing to test the flags to allow the testing to be tested.


-- 
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] chethanuk-plutoflume commented on a diff in pull request #25205: Convert RDS Export Sample DAG to System Test (AIP-47)

Posted by GitBox <gi...@apache.org>.
chethanuk-plutoflume commented on code in PR #25205:
URL: https://github.com/apache/airflow/pull/25205#discussion_r926085850


##########
airflow/providers/amazon/aws/operators/rds.py:
##########
@@ -152,25 +155,27 @@ def execute(self, context: 'Context') -> str:
                 Tags=self.tags,
             )
             create_response = json.dumps(create_instance_snap, default=str)
-            self._await_status(
-                'instance_snapshot',
-                self.db_snapshot_identifier,
-                wait_statuses=['creating'],
-                ok_statuses=['available'],
-            )
+            if self.wait_for_completion:
+                self._await_status(
+                    'instance_snapshot',
+                    self.db_snapshot_identifier,
+                    wait_statuses=['creating'],
+                    ok_statuses=['available'],
+                )
         else:
             create_cluster_snap = self.hook.conn.create_db_cluster_snapshot(
                 DBClusterIdentifier=self.db_identifier,
                 DBClusterSnapshotIdentifier=self.db_snapshot_identifier,
                 Tags=self.tags,
             )
             create_response = json.dumps(create_cluster_snap, default=str)
-            self._await_status(
-                'cluster_snapshot',
-                self.db_snapshot_identifier,
-                wait_statuses=['creating'],
-                ok_statuses=['available'],
-            )
+            if self.wait_for_completion:

Review Comment:
   Instead of duplicating may be move this outside if [to line 179]?



-- 
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] potiuk merged pull request #25205: Convert RDS Export Sample DAG to System Test (AIP-47)

Posted by GitBox <gi...@apache.org>.
potiuk merged PR #25205:
URL: https://github.com/apache/airflow/pull/25205


-- 
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] ferruzzi commented on a diff in pull request #25205: Convert RDS Export Sample DAG to System Test (AIP-47)

Posted by GitBox <gi...@apache.org>.
ferruzzi commented on code in PR #25205:
URL: https://github.com/apache/airflow/pull/25205#discussion_r926090756


##########
airflow/providers/amazon/aws/operators/rds.py:
##########
@@ -152,25 +155,27 @@ def execute(self, context: 'Context') -> str:
                 Tags=self.tags,
             )
             create_response = json.dumps(create_instance_snap, default=str)
-            self._await_status(
-                'instance_snapshot',
-                self.db_snapshot_identifier,
-                wait_statuses=['creating'],
-                ok_statuses=['available'],
-            )
+            if self.wait_for_completion:
+                self._await_status(
+                    'instance_snapshot',
+                    self.db_snapshot_identifier,
+                    wait_statuses=['creating'],
+                    ok_statuses=['available'],
+                )
         else:
             create_cluster_snap = self.hook.conn.create_db_cluster_snapshot(
                 DBClusterIdentifier=self.db_identifier,
                 DBClusterSnapshotIdentifier=self.db_snapshot_identifier,
                 Tags=self.tags,
             )
             create_response = json.dumps(create_cluster_snap, default=str)
-            self._await_status(
-                'cluster_snapshot',
-                self.db_snapshot_identifier,
-                wait_statuses=['creating'],
-                ok_statuses=['available'],
-            )
+            if self.wait_for_completion:

Review Comment:
   Totally fair.   I'll make the change in both `RdsCreateDbSnapshotOperator` and `RdsCopyDbSnapshotOperator`



-- 
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