WebSocket streaming alignment & deprecation fixes#268
WebSocket streaming alignment & deprecation fixes#268
Conversation
Made-with: Cursor
Made-with: Cursor
Made-with: Cursor
Made-with: Cursor
Made-with: Cursor
Made-with: Cursor
Made-with: Cursor
Made-with: Cursor
Made-with: Cursor
…and PymiloClient Updated the close method documentation for clarity and streamlined the destructor logic to remove unnecessary try-except blocks. This enhances readability and ensures proper resource management during object destruction.
…mments Improved the cleanup process in the WebSocketClientCommunicator by ensuring the event loop is checked before attempting to disconnect. Additionally, clarified comments in the test file regarding the loading of JSON data for better understanding.
sadrasabouri
left a comment
sadrasabouri
left a comment
There was a problem hiding this comment.
I left two comments which is applicable throughout the PR. Can you please fix them and request review me again?
sadrasabouri
left a comment
sadrasabouri
left a comment
There was a problem hiding this comment.
I reviewed it again and it required some more changes throught the PR.
| :param client_id: ID of the client | ||
| :param model_id: ID of the model | ||
| :param model: serialized model content | ||
| :return: True if upload was successful, False otherwise |
There was a problem hiding this comment.
We also remove the returns in the new docstring formats.
| response = self.loop.run_until_complete( | ||
| self.send_message("remove_client", {"client_id": client_id}) | ||
| ) | ||
| return "error" not in response |
There was a problem hiding this comment.
self._check_response_error(response) ?
| Register ML model in the PyMiloServer. | ||
|
|
||
| :param client_id: id of the client who owns the model | ||
| :type client_id: str |
There was a problem hiding this comment.
you missed this from the last round of edition ig.
| "ml_model_id": model_id, | ||
| }) | ||
| ) | ||
| return "error" not in response |
There was a problem hiding this comment.
self._check_response_error(response) ?
There was a problem hiding this comment.
this happens other places as well. Please fix it all over the PR.
|
|
||
| :param websocket: the WebSocket connection to remove. | ||
| :type websocket: webSocket | ||
| :type websocket: WebSocket |
| payload["client_id"], | ||
| payload["ml_model_id"], | ||
| ), | ||
| "message": f"Download request from client: {client_id} for model: {ml_model_id}", |
There was a problem hiding this comment.
move this to params if possible.
| result = self._ps.execute_model(call_payload) | ||
| return { | ||
| "message": "Attribute call executed.", | ||
| "message": f"Attribute call request from client: {client_id} for model: {ml_model_id}", |
2 participants
Reference Issues/PRs
What does this implement/fix? Explain your changes.
scenario4test for access control features on both protocolsget_allowancereturning 404 on empty allowancesPymiloClientandWebSocketClientCommunicatorAny other comments?