Fokko commented on code in PR #6159:
URL: https://github.com/apache/iceberg/pull/6159#discussion_r1025777318
##########
python/pyiceberg/utils/config.py:
##########
@@ -44,7 +44,7 @@ def merge_config(lhs: RecursiveDict, rhs: RecursiveDict) ->
RecursiveDict:
if isinstance(lhs_value, dict) and isinstance(rhs_value, dict):
# If they are both dicts, then we have to go deeper
new_config[rhs_key] = merge_config(lhs_value, rhs_value)
- else:
+ elif isinstance(lhs_value, str) and isinstance(rhs_value, str):
Review Comment:
I think we're almost there, but we changed the behavior a bit here. Here we
check if both the lhs and rhs are set, but one or both of them can be None. How
about:
```
else:
# Take the non-null value, with precedence on rhs
new_config[rhs_key] = lhs_value or rhs_value
```
and then we can remove the `_coalesce` function (it is only used in one
place).
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
