dstandish commented on a change in pull request #19665:
URL: https://github.com/apache/airflow/pull/19665#discussion_r759873574



##########
File path: airflow/providers/amazon/aws/hooks/redshift.py
##########
@@ -65,9 +81,9 @@ def cluster_status(self, cluster_identifier: str) -> str:
         """
         try:
             response = 
self.get_conn().describe_clusters(ClusterIdentifier=cluster_identifier)['Clusters']
-            return response[0]['ClusterStatus'] if response else None
+            return RedshiftClusterStates(response[0]['ClusterStatus']) if 
response else None

Review comment:
       > IMO if there are breaking API changes and the API starts returning 
different states than you expect it's going to break your code whether you're 
using strings or an enum, so it's a moot point
   
   Not necessarily.  This hook method is just returning status.  If new 
statuses appear (or if you missed one in your research), it won't break your 
code, unless you force it into an enum that doesn't know the new status.  And 
it's far from certain that a new status would break your operator.  If you are 
waiting for your cluster to become `available` and there's a new status 
`kindof-almost-available` well that's still not available so your logic is 
unaffected -- unless of course you are forcing it into an enum and then your 
code breaks unnecessarily.  That's the scenario that makes me wary of this 
approach.




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


Reply via email to