Dev-iL commented on code in PR #58062:
URL: https://github.com/apache/airflow/pull/58062#discussion_r2536675170


##########
airflow-core/src/airflow/models/connection.py:
##########
@@ -339,6 +339,24 @@ def get_uri(self) -> str:
 
         return uri
 
+    def _safe_decrypt(self, fld: str) -> str:
+        """Safely decrypt and decode a field with proper error handling."""
+        from cryptography.fernet import InvalidToken
+
+        fernet = get_fernet()
+        try:
+            as_bytes = bytes(fld, "utf-8")
+        except (TypeError, UnicodeEncodeError):
+            return fld

Review Comment:
   ### Context:
   This is the issue in question:
   ```none
   airflow-core/src/airflow/models/connection.py:377: error: Incompatible types 
in assignment (expression has type "str | None", variable
   has type "str")  [assignment]
                   extra_val = self._extra
                               ^~~~~~~~~~~
   ```
   The solution proposed before my suggestion was this:
   ```python
               try:
                   return fernet.decrypt(bytes(self._password, 
"utf-8")).decode()
               except Exception as e:
                   from cryptography.fernet import InvalidToken
   
                   if isinstance(e, InvalidToken):
                       return self._password
                   raise
   ```
   I suggested the helper method for two reasons:
   1. We have the `fernet.decrypt(bytes(self.*, "utf-8")).decode()` patterns in 
more than one place.
   2. `except Exception` is too broad
   
   Therefore, it made sense to me to handle the exceptions of this fairly 
complex expression in one place, while improving the exception handling 
resolution.
   
   Adding logs sounds useful and is exactly the sort of benefit we get from 
this helper function.



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