-
Notifications
You must be signed in to change notification settings - Fork 42
v0.0.4b for exospherehost pypi sdk #122
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
v0.0.4b for exospherehost pypi sdk #122
Conversation
…ust poll interval in Runtime class constructor
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe changes update the ExosphereHost Python SDK with expanded usage documentation, restructured public API exports, and a sample usage script. The Changes
Sequence Diagram(s)sequenceDiagram
participant UserScript
participant Runtime
participant BaseNode
participant StateManager
UserScript->>Runtime: Initialize (namespace, state_manager_uri, key, ...)
UserScript->>Runtime: connect([BaseNode])
UserScript->>Runtime: start()
loop Polling
Runtime->>StateManager: POST /enqueue (with x-api-key)
StateManager-->>Runtime: Response (state batch)
Runtime->>BaseNode: execute(inputs)
BaseNode-->>Runtime: result dict
Runtime->>StateManager: POST /notify_executed (with x-api-key)
alt On error
Runtime->>StateManager: POST /notify_errored (with x-api-key)
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🧰 Additional context used🪛 markdownlint-cli2 (0.17.2)python-sdk/README.md4-4: Headings should be surrounded by blank lines (MD022, blanks-around-headings) 5-5: Trailing spaces (MD009, no-trailing-spaces) 23-23: Headings should be surrounded by blank lines (MD022, blanks-around-headings) 24-24: Files should end with a single newline character (MD047, single-trailing-newline) 🔇 Additional comments (3)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 8
🔭 Outside diff range comments (1)
python-sdk/exospherehost/runtime.py (1)
96-107: Ensuretask_done()is always called.The
task_done()call should be in a finally block to ensure it's called even if an unexpected exception occurs. This prevents the queue from getting stuck.async def _worker(self): while True: state = await self._state_queue.get() - try: - node = self._node_mapping[state["node_name"]] - outputs = await node.execute(state["inputs"]) # type: ignore - await self._notify_executed(state["state_id"], outputs) - except Exception as e: - await self._notify_errored(state["state_id"], str(e)) - - self._state_queue.task_done() # type: ignore + try: + node = self._node_mapping[state["node_name"]] + outputs = await node.execute(state["inputs"]) # type: ignore + await self._notify_executed(state["state_id"], outputs) + except Exception as e: + await self._notify_errored(state["state_id"], str(e)) + finally: + self._state_queue.task_done() # type: ignore
♻️ Duplicate comments (1)
python-sdk/exospherehost/runtime.py (1)
75-82: Apply consistent error handling.Similar to
_notify_executed, this method should handle non-200 responses.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
python-sdk/README.md(1 hunks)python-sdk/exospherehost/__init__.py(1 hunks)python-sdk/exospherehost/_version.py(1 hunks)python-sdk/exospherehost/node/BaseNode.py(2 hunks)python-sdk/exospherehost/node/__init__.py(0 hunks)python-sdk/exospherehost/runtime.py(3 hunks)python-sdk/pyproject.toml(1 hunks)python-sdk/sample.py(1 hunks)
💤 Files with no reviewable changes (1)
- python-sdk/exospherehost/node/init.py
🧰 Additional context used
🧬 Code Graph Analysis (2)
python-sdk/exospherehost/node/BaseNode.py (1)
python-sdk/sample.py (1)
execute(5-7)
python-sdk/exospherehost/runtime.py (1)
python-sdk/exospherehost/node/BaseNode.py (2)
BaseNode(5-17)get_unique_name(14-17)
🪛 LanguageTool
python-sdk/README.md
[grammar] ~2-~2: Use articles correctly
Context: # ExosphereHost Python SDK This SDK is official python SDK for ExosphereHost and intera...
(QB_NEW_EN_OTHER_ERROR_IDS_11)
[grammar] ~2-~2: Use proper capitalization
Context: ... for ExosphereHost and interacting with exospherehost. ## Node Creation You can simply connec...
(QB_NEW_EN_OTHER_ERROR_IDS_6)
[grammar] ~5-~5: Use articles correctly
Context: ...Node Creation You can simply connect to exosphere state manager and start creating your n...
(QB_NEW_EN_OTHER_ERROR_IDS_11)
[grammar] ~5-~5: There might be a problem here.
Context: ... start creating your nodes, as shown in sample below: python from exospherehost import Runtime, BaseNode import os class SampleNode(BaseNode): async def execute(self, inputs): print(inputs) return {"message": "success"} runtime = Runtime("SampleNamespace", os.getenv("EXOSPHERE_STATE_MANAGER_URI", "http://localhost:8000"), os.getenv("EXOSPHERE_API_KEY", "")) runtime.connect([SampleNode()]) runtime.start() ## Support For first party ...
(QB_NEW_EN_MERGED_MATCH)
[grammar] ~23-~23: Use hyphens correctly
Context: ...)]) runtime.start() ``` ## Support For first party support and questions do not hesit...
(QB_NEW_EN_OTHER_ERROR_IDS_29)
[grammar] ~23-~23: Use a period to end declarative sentences
Context: ...s do not hesitate to reach out to us at nivedit@exosphere.host
(QB_NEW_EN_OTHER_ERROR_IDS_25)
🪛 markdownlint-cli2 (0.17.2)
python-sdk/README.md
4-4: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
5-5: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
22-22: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
23-23: Bare URL used
(MD034, no-bare-urls)
23-23: Files should end with a single newline character
(MD047, single-trailing-newline)
🔇 Additional comments (10)
python-sdk/exospherehost/_version.py (1)
1-1: LGTM! Version update is consistent with the broader refactoring.The change from uppercase
VERSIONto lowercaseversionaligns with Python naming conventions for variables, and the version bump to "0.0.4b" is appropriate for this beta release.python-sdk/pyproject.toml (1)
28-28: LGTM! Dynamic version reference updated correctly.The change from
exospherehost.__version__toexospherehost._version.versioncorrectly aligns with the version management refactoring and maintains consistency across the build configuration.python-sdk/sample.py (1)
9-9: Consider security implications of empty API key default.The default empty string for
EXOSPHERE_API_KEYcould lead to authentication issues or security vulnerabilities. Consider requiring the API key or providing a more explicit error message when it's missing.-runtime = Runtime("SampleNamespace", os.getenv("EXOSPHERE_STATE_MANAGER_URI", "http://localhost:8000"), os.getenv("EXOSPHERE_API_KEY", "")) +api_key = os.getenv("EXOSPHERE_API_KEY") +if not api_key: + raise ValueError("EXOSPHERE_API_KEY environment variable is required") +runtime = Runtime("SampleNamespace", os.getenv("EXOSPHERE_STATE_MANAGER_URI", "http://localhost:8000"), api_key)python-sdk/exospherehost/node/BaseNode.py (2)
2-2: LGTM! Import addition supports enhanced type annotations.The addition of
Anyto the typing imports is necessary for the updated method signature and improves type safety.
11-11: Excellent improvement to the abstract method signature.The updated
executemethod signature with explicit type annotations (inputs: dict[str, Any]and return typedict[str, Any]) provides clear guidance for implementers and improves type safety throughout the SDK.python-sdk/exospherehost/__init__.py (1)
1-7: LGTM! Clean public API definition.The restructured imports and explicit
__all__declaration properly define the SDK's public interface. The backward compatibility forVERSIONis maintained while following the more conventional__version__pattern.python-sdk/exospherehost/runtime.py (4)
10-10: Consider the performance impact of reduced polling interval.The default
poll_intervalwas reduced from 10 to 1 second, which will significantly increase the polling frequency. This could lead to increased API calls and resource consumption. Consider if this aggressive polling is necessary for all use cases, or if it should be configurable based on the workload characteristics.
38-42: Good API simplification.Converting
connectto synchronous improves the developer experience since this method doesn't perform any I/O operations that would benefit from being asynchronous.
44-56: Proper API authentication and error handling implemented.The addition of the
x-api-keyheader and explicit error handling for non-200 responses improves the robustness of the API calls.
109-116: Well-structured async task management.The separation of the enqueue poller and worker tasks with
asyncio.gatherprovides clean concurrent execution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (3)
python-sdk/exospherehost/runtime.py (3)
60-70: Error handling successfully implemented as previously suggested.The try-except block now properly handles exceptions from
_enqueue_call, preventing the enqueue task from crashing.
72-94: Consistent error handling implemented across notification methods.Both
_notify_executedand_notify_errorednow have consistent error handling with logging for non-200 responses, as previously suggested.
130-131: Known limitation:asyncio.run()incompatibility with existing event loops.As previously noted,
asyncio.run()will fail if called from within an existing event loop. This limitation should be documented or addressed with alternative implementation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
python-sdk/exospherehost/runtime.py(3 hunks)
🔇 Additional comments (3)
python-sdk/exospherehost/runtime.py (3)
4-8: Good improvements to imports and logging setup.The more specific import path for
BaseNodeand the addition of proper logging infrastructure are welcome improvements.
48-58: Well-implemented API authentication and error handling.Good additions:
- API key authentication via headers
- Error logging while still returning the response for caller handling
115-119: Critical fix: Added missingtask_done()call.Excellent addition of
self._state_queue.task_done()- this was a critical omission that could have caused queue blocking and memory issues. The key change from "id" to "state_id" correctly aligns with the API schema.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (2)
python-sdk/exospherehost/runtime.py (2)
12-12: Consider the impact of the aggressive default polling interval.The default
poll_intervalof 1 second could lead to excessive API calls and potential rate limiting issues in production. Consider using a more conservative default (e.g., 5-10 seconds) or clearly documenting the implications.
40-44: Breaking change:connectis now synchronous.The
connectmethod has been changed from async to sync. This is a breaking API change that should be clearly documented in the changelog and migration guide.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
python-sdk/exospherehost/runtime.py(3 hunks)
🔇 Additional comments (6)
python-sdk/exospherehost/runtime.py (6)
46-58: Good addition of authentication and error logging.The inclusion of the API key header and error logging for non-200 responses improves security and debugging capabilities.
60-70: Excellent implementation of error handling.The addition of try-except block ensures the enqueue task continues running despite transient errors, addressing the concern raised in previous reviews.
72-83: Consistent error handling implemented.The addition of error logging for non-200 responses provides better visibility into failures, addressing the consistency concern from previous reviews.
84-94: Good consistency in error handling across notification methods.Both notification methods now have uniform error handling and authentication patterns.
119-119: Good addition of queue task completion.Adding
task_done()ensures proper queue management and allows for join operations if needed.
115-117: Confirmed:state_idusage matches the API
The SDK’s switch fromstate["id"]tostate["state_id"]aligns with the PydanticResponseStateModel(which definesstate_id) and theexecuted_state/errored_statecontrollers expectingstate_id. No further changes needed.
No description provided.