Taragolis commented on code in PR #35048:
URL: https://github.com/apache/airflow/pull/35048#discussion_r1365166781


##########
airflow/utils/log/secrets_masker.py:
##########
@@ -279,9 +279,9 @@ def _redact(self, item: Redactable, name: str | None, 
depth: int, max_depth: int
         # but it caused infinite recursion, so we need to cast it to str first.
         except Exception as exc:
             log.warning(
-                "Unable to redact %r, please report this via 
<https://github.com/apache/airflow/issues>. "
+                "Unable to redact %s, please report this via 
<https://github.com/apache/airflow/issues>. "
                 "Error was: %s: %s",
-                item,
+                repr(item),

Review Comment:
   In general I think it is one of possible way to solve the issue. In the 
current implementation we pass into arguments which unable to masked and the 
error happen again, as result recursion happen. 
   
   Maybe we even would like to build warning message outside of the logger: 
   
   ```python
           except Exception as exc:
               warning_msg = (
                   r"Unable to redact {item!r}, please report this via 
<https://github.com/apache/airflow/issues>. "
                   "Error was: {type(exc).__name__}: {exc}",
               )
               log.warning(warning_msg)
               return item
   ```
   
   Another option: use logger without `mask_secrets` filter here
   
   @uranusjr WDYT?



##########
airflow/utils/db_cleanup.py:
##########
@@ -182,7 +182,7 @@ def _do_delete(*, query, orm_model, skip_archive, session):
     metadata = reflect_tables([orm_model.name, target_table_name], session)
     source_table = metadata.tables[orm_model.name]
     target_table = metadata.tables[target_table_name]
-    logger.debug("rows moved; purging from %s", source_table.name)
+    logger.debug("rows moved; purging from %s", str(source_table.name))

Review Comment:
   Are we sure that we want to include this fix in this PR? Because it seems a 
bit hacky here, and just hide actual problem



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