Adding APIs to Register Runtime and Nodes #135
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe changes refactor the node registration and startup flow in the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 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🧬 Code Graph Analysis (2)state-manager/app/models/register_nodes_request.py (4)
state-manager/app/models/register_nodes_response.py (5)
🔇 Additional comments (7)
✨ 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. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (2)
python-sdk/exospherehost/runtime.py (2)
137-155: Update docstring to reflect the method's new purpose.The docstring still refers to "connecting" nodes, but the method is now
_registerand handles registration.async def _register(self, nodes: List[BaseNode]): """ - Connect nodes to the runtime. + Register nodes with the runtime. - This method validates and registers the provided nodes with the runtime. - The nodes will be available for execution when the runtime starts. + This method validates the provided nodes, updates internal mappings, + and registers them with the state manager service. Args: nodes (List[BaseNode]): List of BaseNode instances to connect Raises: ValueError: If any node does not inherit from BaseNode + RuntimeError: If node registration fails """
284-299: Update docstring to remove outdated RuntimeError description.The docstring still mentions raising RuntimeError when "not connected", but this check has been removed.
async def _start(self, nodes: List[BaseNode]): """ Start the runtime execution. This method starts the enqueue polling task and spawns worker tasks to process states from the queue. Raises: - RuntimeError: If the runtime is not connected (no nodes registered) + ValueError: If any node does not inherit from BaseNode + RuntimeError: If node registration fails """
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
python-sdk/exospherehost/runtime.py(5 hunks)
🔇 Additional comments (3)
python-sdk/exospherehost/runtime.py (3)
107-109: LGTM!The new endpoint getter follows the established pattern for other endpoint methods in the class.
301-316: Good API simplification!The consolidated approach of passing nodes directly to
start()eliminates the need for a separate connection step, making the API more intuitive.
137-155: Breaking API change requires user migration.This refactoring changes the public API by requiring nodes to be passed to
start()instead of callingconnect()first. Ensure that:
- This breaking change is documented in release notes
- Migration guide is provided for existing users
- Examples and documentation are updated
Also applies to: 284-299, 301-316
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
state-manager/app/controller/register_nodes.py(1 hunks)state-manager/app/main.py(2 hunks)state-manager/app/models/db/registered_node.py(1 hunks)state-manager/app/models/register_nodes_request.py(1 hunks)state-manager/app/models/register_nodes_response.py(1 hunks)state-manager/app/routes.py(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
state-manager/app/models/register_nodes_response.py (6)
state-manager/app/models/create_models.py (3)
ResponseStateModel(12-16)RequestStateModel(7-9)CreateResponseModel(23-25)state-manager/app/models/enqueue_response.py (2)
StateModel(6-10)EnqueueResponseModel(13-18)state-manager/app/models/db/state.py (1)
State(7-14)state-manager/app/models/enqueue_request.py (1)
EnqueueRequestModel(4-6)state-manager/app/models/errored_models.py (1)
ErroredResponseModel(9-10)state-manager/app/models/executed_models.py (1)
ExecutedResponseModel(9-10)
state-manager/app/routes.py (4)
state-manager/app/models/register_nodes_request.py (1)
RegisterNodesRequestModel(12-15)state-manager/app/models/register_nodes_response.py (1)
RegisterNodesResponseModel(10-13)state-manager/app/controller/register_nodes.py (1)
register_nodes(11-67)state-manager/app/utils/check_secret.py (1)
check_api_key(15-19)
🔇 Additional comments (10)
state-manager/app/models/register_nodes_request.py (1)
5-9: LGTM! Well-structured node registration model.The
NodeRegistrationModelfollows consistent patterns with other Pydantic models in the codebase and appropriately usesdict[str, Any]for JSON schema fields. Field descriptions are clear and all required fields are properly annotated.state-manager/app/main.py (2)
24-24: LGTM! Proper model import.The import of
RegisteredNodeis correctly added alongside other database models.
40-40: LGTM! Correct Beanie ODM integration.Adding
RegisteredNodeto the document models list properly integrates the new model with the database initialization process.state-manager/app/models/db/registered_node.py (1)
6-12: LGTM! Well-designed database model.The
RegisteredNodemodel is properly structured with appropriate field types and comprehensive metadata. The use ofdict[str, Any]for schema fields is correct for storing JSON schemas, and the runtime tracking fields enable proper node ownership management.state-manager/app/controller/register_nodes.py (2)
11-18: LGTM! Well-structured function signature and initialization.The function signature properly accepts all necessary parameters with correct type annotations. The initial logging with request ID tracking is a good practice.
56-67: LGTM! Proper response construction and error handling.The response model construction correctly maps the processed data, and the error handling appropriately logs errors with request ID context while preserving the original exception for upstream handling.
state-manager/app/models/register_nodes_response.py (1)
5-7: LGTM! Appropriate response model structure.The
RegisteredNodeModelcorrectly provides essential node identification without unnecessarily echoing back the full schema data, which is a good practice for response models.state-manager/app/routes.py (3)
24-26: LGTM!The imports are correctly structured and follow the established pattern in the file for bringing in the necessary components for the new node registration route.
131-132: Good formatting improvement.The additional newlines provide better visual separation between route handlers, improving code readability.
134-150: Excellent implementation following established patterns.The new route handler correctly follows all the established patterns in the file:
- Proper FastAPI decorator configuration with appropriate status code for upsert operations
- Consistent parameter structure and API key validation
- Same request ID extraction and logging patterns
- Proper error handling structure
The use of HTTP 200 OK is appropriate since the controller performs upsert operations (update existing or create new nodes).
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
state-manager/app/controller/register_nodes.py (1)
32-33: Remove unusedinserted_nodeand clean up schema type ignoresThe
# type: ignorecomments suggest potential type mismatches with the Beanie Set operator usage. This could indicate issues with the schema field definitions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
state-manager/app/controller/register_nodes.py(1 hunks)
🔇 Additional comments (3)
state-manager/app/controller/register_nodes.py (3)
1-8: LGTM!The imports are well-organized and appropriate for the node registration functionality. The separation of request/response models, database models, and utilities follows good architectural practices.
11-17: LGTM!The function signature is well-designed with proper typing, clear parameter separation, and good observability through the request ID parameter. The try-catch structure and initial logging provide good error handling foundation.
57-68: LGTM!The response construction is clean and well-structured. The error handling appropriately logs errors while preserving the original exception for upstream handling. The success logging provides good observability.
1 participant
No description provided.