feat: DH-21667: Add Python wrapper for Configuration.#7696
feat: DH-21667: Add Python wrapper for Configuration.#7696cpwright wants to merge 6 commits intodeephaven:mainfrom
Conversation
cpwright
requested review from
chipkent,
jmao-denver and
rcaudy
as code owners
No docs changes detected for eb557ea |
cpwright
added
NoDocumentationNeeded
ReleaseNotesNeeded
There was a problem hiding this comment.
Overall the interface feels very Java-ish vs. Pythonic. I wonder if we should make it more convenient for the users to return a dict-like object (i.e. implement the mapping protocol). On the other hand, it is not very different from Python's ConfigParser except the mapping protocol implementation part.
| def has_property(self, property_name: str) -> bool: | ||
| """Check if a configuration property is defined. |
There was a problem hiding this comment.
It seems to me that we probably want something like keys(), values() and items() if one of the primary use cases of this class is for a Python user to view the configuration of the server.
Co-authored-by: Jianfeng Mao <4297243+jmao-denver@users.noreply.github.com>
cpwright
left a comment
cpwright
left a comment
There was a problem hiding this comment.
I (Claude) added the mapping protocol so it can be used in a more pythonic way.
jmao-denver
left a comment
jmao-denver
left a comment
There was a problem hiding this comment.
since we have chosen to implement the mapping protocol, might as well do a more complete job.
Co-authored-by: Jianfeng Mao <4297243+jmao-denver@users.noreply.github.com>
2 participants
No description provided.