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/22 20:36:49 UTC
[GitHub] [airflow] josh-fell commented on a diff in pull request #25857: Add redshift create cluster snapshot operator
josh-fell commented on code in PR #25857:
URL: https://github.com/apache/airflow/pull/25857#discussion_r951885486
##########
airflow/providers/amazon/aws/operators/redshift_cluster.py:
##########
@@ -242,6 +243,63 @@ def execute(self, context: 'Context'):
self.log.info(cluster)
+class RedshiftCreateClusterSnapshotOperator(BaseOperator):
+ """
+ Creates a manual snapshot of the specified cluster. The cluster must be in the available state
+
+ :param snapshot_identifier: A unique identifier for the snapshot that you are requesting
+ :param cluster_identifier: The cluster identifier for which you want a snapshot
+ :param retention_period: The number of days that a manual snapshot is retained
+ :param wait_for_completion: Whether wait for cluster to be in ``available`` state
+ :param poll_interval: Time (in seconds) to wait between two consecutive calls to check cluster state
+ """
+
+ def __init__(
+ self,
+ *,
+ snapshot_identifier: str,
+ cluster_identifier: str,
+ retention_period: int = -1,
+ wait_for_completion: bool = False,
+ poll_interval: float = 5.0,
+ aws_conn_id: str = "aws_default",
Review Comment:
Can you add this parameter to the docstring please?
##########
airflow/providers/amazon/aws/operators/redshift_cluster.py:
##########
@@ -242,6 +243,63 @@ def execute(self, context: 'Context'):
self.log.info(cluster)
+class RedshiftCreateClusterSnapshotOperator(BaseOperator):
+ """
+ Creates a manual snapshot of the specified cluster. The cluster must be in the available state
+
+ :param snapshot_identifier: A unique identifier for the snapshot that you are requesting
+ :param cluster_identifier: The cluster identifier for which you want a snapshot
+ :param retention_period: The number of days that a manual snapshot is retained
+ :param wait_for_completion: Whether wait for cluster to be in ``available`` state
+ :param poll_interval: Time (in seconds) to wait between two consecutive calls to check cluster state
+ """
+
+ def __init__(
+ self,
+ *,
+ snapshot_identifier: str,
+ cluster_identifier: str,
+ retention_period: int = -1,
Review Comment:
It might be useful to make a note about what the default value is here and what -1 means. I assume it means to never delete the snapshot, but better to be explicit than implicit. WDYT?
##########
airflow/providers/amazon/aws/operators/redshift_cluster.py:
##########
@@ -242,6 +243,63 @@ def execute(self, context: 'Context'):
self.log.info(cluster)
+class RedshiftCreateClusterSnapshotOperator(BaseOperator):
+ """
+ Creates a manual snapshot of the specified cluster. The cluster must be in the available state
+
+ :param snapshot_identifier: A unique identifier for the snapshot that you are requesting
+ :param cluster_identifier: The cluster identifier for which you want a snapshot
+ :param retention_period: The number of days that a manual snapshot is retained
+ :param wait_for_completion: Whether wait for cluster to be in ``available`` state
+ :param poll_interval: Time (in seconds) to wait between two consecutive calls to check cluster state
+ """
+
+ def __init__(
+ self,
+ *,
+ snapshot_identifier: str,
+ cluster_identifier: str,
+ retention_period: int = -1,
+ wait_for_completion: bool = False,
+ poll_interval: float = 5.0,
+ aws_conn_id: str = "aws_default",
+ **kwargs,
+ ):
+ super().__init__(**kwargs)
+ self.snapshot_identifier = snapshot_identifier
+ self.cluster_identifier = cluster_identifier
+ self.retention_period = retention_period
+ self.wait_for_completion = wait_for_completion
+ self.poll_interval = poll_interval
+ self.redshift_hook = RedshiftHook(aws_conn_id=aws_conn_id)
+
+ def execute(self, context: Context) -> Any:
Review Comment:
```suggestion
def execute(self, context: "Context") -> Any:
```
##########
airflow/providers/amazon/aws/operators/redshift_cluster.py:
##########
@@ -242,6 +243,63 @@ def execute(self, context: 'Context'):
self.log.info(cluster)
+class RedshiftCreateClusterSnapshotOperator(BaseOperator):
+ """
+ Creates a manual snapshot of the specified cluster. The cluster must be in the available state
+
+ :param snapshot_identifier: A unique identifier for the snapshot that you are requesting
+ :param cluster_identifier: The cluster identifier for which you want a snapshot
+ :param retention_period: The number of days that a manual snapshot is retained
+ :param wait_for_completion: Whether wait for cluster to be in ``available`` state
+ :param poll_interval: Time (in seconds) to wait between two consecutive calls to check cluster state
+ """
+
+ def __init__(
+ self,
+ *,
+ snapshot_identifier: str,
+ cluster_identifier: str,
+ retention_period: int = -1,
+ wait_for_completion: bool = False,
+ poll_interval: float = 5.0,
+ aws_conn_id: str = "aws_default",
+ **kwargs,
+ ):
+ super().__init__(**kwargs)
+ self.snapshot_identifier = snapshot_identifier
+ self.cluster_identifier = cluster_identifier
+ self.retention_period = retention_period
+ self.wait_for_completion = wait_for_completion
+ self.poll_interval = poll_interval
+ self.redshift_hook = RedshiftHook(aws_conn_id=aws_conn_id)
+
+ def execute(self, context: Context) -> Any:
+ cluster_state = self.redshift_hook.cluster_status(cluster_identifier=self.cluster_identifier)
+ if cluster_state != "available":
+ raise AirflowException(
+ f"Redshift cluster must be in available state."
Review Comment:
```suggestion
"Redshift cluster must be in available state. "
```
Nit. The f-string is not _technically_ needed and just adding a space between the concatenated lines.
--
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