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 sort of 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.
-------
HOWEVER, the mypy violation mentioned above is currently the only one in
this file (i.e. raised for `_extra` but not for `_password`) - so perhaps it
can/should be handled at the model level instead?
--
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]
