[GitHub] [airflow] uranusjr commented on a change in pull request #16424: Switch to built-in data structures in SecretsMasker

[GitBox <gi...@apache.org>]

uranusjr commented on a change in pull request #16424:
URL: https://github.com/apache/airflow/pull/16424#discussion_r650578039



##########
File path: airflow/utils/log/secrets_masker.py
##########
@@ -196,12 +195,11 @@ def redact(self, item: "RedactableItem", name: str = None) -> "RedactableItem":
             elif isinstance(item, (tuple, set)):
                 # Turn set in to tuple!
                 return tuple(self.redact(subval) for subval in item)
-            elif isinstance(item, io.IOBase):
-                return item
-            elif isinstance(item, Iterable):
+            elif isinstance(item, list):
                 return list(self.redact(subval) for subval in item)
             else:
                 return item
+        # I think this should never happen, but it does not hurt to leave it just in case

Review comment:
       I think it’s technically possible if the user passes in some kind of custom subclass (e.g. `class MyList(list):` that overrides some weird stuff), so yeah let’s keep the exception handler there, but I don’t think it’s worthwhile to have a test for it.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org