pierrejeambrun commented on code in PR #53943:
URL: https://github.com/apache/airflow/pull/53943#discussion_r2245210680
##########
task-sdk/src/airflow/sdk/execution_time/secrets_masker.py:
##########
@@ -292,6 +313,83 @@ def _redact(self, item: Redactable, name: str | None,
depth: int, max_depth: int
)
return item
+ def _merge(
+ self,
+ new_item: Redacted,
+ old_item: Redactable,
+ name: str | None,
+ depth: int,
+ max_depth: int,
+ force_sensitive: bool = False,
+ ) -> Redacted:
+ """Merge a redacted item with its original unredacted counterpart."""
+ if depth > max_depth:
+ if isinstance(new_item, str) and new_item == "***":
+ return old_item
+ return new_item
+
+ try:
+ # Determine if we should treat this as sensitive
+ is_sensitive = force_sensitive or (name is not None and
should_hide_value_for_key(name))
+
+ if isinstance(new_item, dict) and isinstance(old_item, dict):
+ merged = {}
+ for key in new_item.keys():
+ if key in old_item:
+ # For dicts, pass the key as name unless we're in
sensitive mode
+ child_name = None if is_sensitive else key
+ merged[key] = self._merge(
+ new_item[key],
+ old_item[key],
+ name=child_name,
+ depth=depth + 1,
+ max_depth=max_depth,
+ force_sensitive=is_sensitive,
+ )
+ else:
+ merged[key] = new_item[key]
+ return merged
+
+ if isinstance(new_item, (list, tuple)) and type(old_item) is
type(new_item):
+ merged_list = []
+ for i in range(len(new_item)):
Review Comment:
I gave it a try, indeed it removes a bit of code but i makes things harder
to understand, symetric logic such as:
```
self._merge(
new_item[i],
old_item[i],
name=None,
depth=depth + 1,
max_depth=max_depth,
force_sensitive=is_sensitive,
)
```
becomes:
```
self._merge(
item,
old_item[i],
name=None,
depth=depth + 1,
max_depth=max_depth,
force_sensitive=is_sensitive,
)
```
It's slightly harder to compare `item` and `old_item[i]`, same for
`isinstance(item, str)`, not super straighforward to know if this is old or new
item.
That's just a matter of taste, and I understand your point. I'll merge like
this to not delay further, but if people feel like this is easier to read, I'm
down to modify 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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
