GayathriSrividya commented on PR #3423: URL: https://github.com/apache/iceberg-python/pull/3423#issuecomment-4966057241
> Thank you for the PR. Im not sure if its a good idea to accept both JSON string and flattened properties. I think we should just pick one. My vote is the flattened properties, we dont really have other patterns of accepting JSON string as config. > > Accepting both makes documentation confusing. and JSON string might contain other arbitrary key/value pairs > > I would also like to keep changes to `pyiceberg/catalog/rest/__init__.py` to a minimum, seems like there are a lot of helper functions that might not be necessary. Hopefully dropping JSON string support will get rid of a lot of the changes. If we truly need to define helper methods, it might be better to add them to `pyiceberg/catalog/rest/auth.py` instead. Thanks for the feedback, I’ve updated this to flattened properties only and removed support for parsing auth as a JSON string. I also kept the change surface minimal in rest init by using the existing auth config flow and removing the JSON auth path/tests. Pushed in 532d0979. -- 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]
