AetherUnbound commented on code in PR #34773:
URL: https://github.com/apache/airflow/pull/34773#discussion_r1347512062


##########
airflow/providers/amazon/aws/hooks/rds.py:
##########
@@ -240,7 +240,7 @@ def get_db_instance_state(self, db_instance_id: str) -> str:
         try:
             response = 
self.conn.describe_db_instances(DBInstanceIdentifier=db_instance_id)
         except self.conn.exceptions.ClientError as e:
-            if e.response["Error"]["Code"] == "DBInstanceNotFoundFault":
+            if e.response["Error"]["Code"] == "DBInstanceNotFound":

Review Comment:
   @vincbeck Right, I mention this in the PR description - the error code 
itself within the `ClientError` is `DBInstanceNotFound`, but the exception is 
`DBInstanceNotFoundFault`. I'm all for catching a more specific exception, but 
the other functions in this file will probably need to be updated as well and I 
wanted to confirm that was the right way forward before making that change 🙂 
   
   > Are there any version changes for SDK/Client/API used now vs earlier?
   
   Not that I'm aware of, I think this has been an issue since we started using 
this DAG! Let me take a look at our logs and get back to you though.



##########
airflow/providers/amazon/aws/hooks/rds.py:
##########
@@ -240,7 +240,7 @@ def get_db_instance_state(self, db_instance_id: str) -> str:
         try:
             response = 
self.conn.describe_db_instances(DBInstanceIdentifier=db_instance_id)
         except self.conn.exceptions.ClientError as e:
-            if e.response["Error"]["Code"] == "DBInstanceNotFoundFault":
+            if e.response["Error"]["Code"] == "DBInstanceNotFound":

Review Comment:
   @vincbeck Right, I mention this in the PR description - the error code 
itself within the `ClientError` is `DBInstanceNotFound`, but the exception is 
`DBInstanceNotFoundFault`. I'm all for catching a more specific exception, but 
the other functions in this file will probably need to be updated as well and I 
wanted to confirm that was the right way forward before making that change 🙂 
   
   > Are there any version changes for SDK/Client/API used now vs earlier?
   
   Not that I'm aware of, I think this has been an issue since we started using 
this DAG! Let me take a look at our logs and get back to you though.



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