Promote Python SDK in sdks/python-sdk from POC to a complete and supported SDK #825
Conversation
- FEATURES.md to outline all available features of the SDK, including client features, authentication, target management, solution management, and more. - GLOSSARY.md to define key terms and concepts used within the SDK, enhancing understanding for new users. - QUICKSTART.md to provide a step-by-step guide for installation and basic usage, including examples for connecting to Symphony, working with targets, deploying solutions, and tracking deployment status.
- Introduced `models.py` containing data structures for Symphony SDK, including specifications for instances, components, and errors. - Added `summary.py` to provide Python translations of Symphony API summary models, including result specifications and summary generation. - Created `types.py` to define essential types and states used in the Symphony COA API, including error handling and constants.
…types and api-client
…etter - Initialized examples/README.md with detailed descriptions of new examples and usage instructions.
Haishi2016
left a comment
Haishi2016
left a comment
There was a problem hiding this comment.
Great PR. Some comments are added.
|
|
||
| return self._handle_response(response) | ||
|
|
||
| def list_targets(self) -> List[Dict[str, Any]]: |
There was a problem hiding this comment.
This is a common pattern in multiple methods: _handle_reuquest() returns Dict[str, Any] while this method returns List[Dict[str,Any]]. Also, should methods like this use types defined in models.py like TargetState?
There was a problem hiding this comment.
This is a common pattern in multiple methods: _handle_reuquest() returns Dict[str, Any] while this method returns List[Dict[str,Any]]
The response type varies between api calls (yes, all JSON but some returns a list, some returns a dict, some returns a list of dicts etc.), so I remember I had trouble determining the return type of a generic method like handle_response that parses a Response object and tries to return a pure python type. Perhaps my approach to this was flawed but I don't know any better 😁
Also, should methods like this use types defined in models.py like TargetState?
In the current state of the software, methods like this return pure python objects. They don't mess with Symphony object models. My initial thought while developing this was: "constructing models should be the sdk user's job". But if you think otherwise, let me know and I could arrange every corresponding method to return Symphony models instead of the Response JSON (e.g. list_targets would return a list of TargetSpec)
There was a problem hiding this comment.
I see. We can keep the current approach as it is. Or, if we want to make it a bit more clearer, maybe use Any? def _handle_response(...) -> Any. Or, have a JSONType: JSONType = Union[Dict[str, Any], List[Dict[str, Any]]]
def _handle_response(...) -> JSONType. And because the method may return None, make it Optional[JsonType]? It's your call.
There was a problem hiding this comment.
Or, have a JSONType: JSONType = Union[Dict[str, Any], List[Dict[str, Any]]]
def _handle_response(...) -> JSONType. And because the method may return None, make it Optional[JsonType]?
I'll try this soon
There was a problem hiding this comment.
implemented your suggestion for handle_response in 90a9cc5
Left the return types of list_* methods as is. Because callers would need to narrow the type before using it as a list, which would be unnecessary friction in my opinion
… test commands with coverage options
2 participants
As mentioned in #824:
This PR aims to promote the existing Python SDK under
sdks/python-sdkfrom a proof-of-concept to a structured, maintainable, and supported SDK.Summary of changes
This PR introduces the following improvements:
Converts the SDK to a proper Python package with a
pyproject.toml, explicit dependency definitions, and reproducible builds.Establishes a clear SDK structure with separated concerns for:
Non-goals
This PR intentionally does not attempt to:
Those concerns are deferred until the SDK is adopted and exercised by users.
Potential Follow-ups
ruffandmypywarnings/errorsNotes For Reviewers
pyproject.tomlas I couldn't quite choose if we use EPL 2.0 or MIT for Eclipse Symphony. (You'll see license headers in source code, I didn't remove them since they're coming from Muto Agent's repository). If someone enlightens me of the license requirements, I could update accordinglypython-sdk. It is still thereMuto Agent's repo. If PyPI package gets published, we'll migrate to that. @Haishi2016