-
Notifications
You must be signed in to change notification settings - Fork 42
Adding code to trigger state creation for a graph #155
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
Adding code to trigger state creation for a graph #155
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughSummary by CodeRabbit
WalkthroughThese changes update the state creation API to require a Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API_Route
participant Controller
participant DB
Client->>API_Route: POST /graph/{graph_name}/states/create
API_Route->>Controller: create_states(namespace_name, graph_name, body, ...)
Controller->>DB: Fetch GraphTemplate by graph_name & namespace_name
alt GraphTemplate found
Controller->>Controller: get_node_template(graph_template, identifier)
alt NodeTemplate found
Controller->>DB: Insert State with identifier, node_name, etc.
DB-->>Controller: State inserted
Controller-->>API_Route: CreateResponseModel
API_Route-->>Client: Response
else NodeTemplate not found
Controller-->>API_Route: 404 NodeTemplate not found
end
else GraphTemplate not found
Controller-->>API_Route: 404 GraphTemplate not found
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Possibly related PRs
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 (1)
✨ 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 (
|
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: 3
🔭 Outside diff range comments (1)
state-manager/app/controller/create_states.py (1)
59-59: Missing identifier field in ResponseStateModel constructorThe ResponseStateModel constructor is missing the new
identifierfield. According to the model definition increate_models.pyline 15, this field should be included.- states=[ResponseStateModel(state_id=str(state.id), node_name=state.node_name, graph_name=state.graph_name, inputs=state.inputs, created_at=state.created_at) for state in newStates] + states=[ResponseStateModel(state_id=str(state.id), node_name=state.node_name, identifier=state.identifier, graph_name=state.graph_name, inputs=state.inputs, created_at=state.created_at) for state in newStates]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
state-manager/app/controller/create_states.py(1 hunks)state-manager/app/models/create_models.py(1 hunks)state-manager/app/models/db/state.py(1 hunks)state-manager/app/routes.py(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: NiveditJain
PR: exospherehost/exospherehost#132
File: state-manager/app/controller/upsert_graph_template.py:11-27
Timestamp: 2025-08-03T16:46:04.030Z
Learning: In the exospherehost codebase, for upsert operations on graph templates, the team prioritizes API idempotency over avoiding race conditions in the database layer implementation. The approach of separate find and insert/update operations is acceptable when the API behavior remains consistent.
🧬 Code Graph Analysis (2)
state-manager/app/models/db/state.py (1)
state-manager/app/models/enqueue_response.py (1)
StateModel(6-10)
state-manager/app/routes.py (3)
state-manager/app/models/create_models.py (2)
CreateResponseModel(25-27)CreateRequestModel(21-22)state-manager/app/utils/check_secret.py (1)
check_api_key(15-19)state-manager/app/controller/create_states.py (1)
create_states(23-64)
🔇 Additional comments (10)
state-manager/app/models/create_models.py (1)
15-15: LGTM!Good addition of the
identifierfield with a clear description. Maintaining bothnode_nameandidentifierin the response model ensures backward compatibility while supporting the new identifier-based workflow.state-manager/app/routes.py (3)
58-58: LGTM!Good API design choice to make the graph context explicit in the URL path. This change aligns well with the requirement to tie state creation to specific graphs.
64-64: LGTM!Function signature correctly updated to include the new
graph_nameparameter from the path.
74-74: LGTM!Controller call properly updated to pass the new
graph_nameparameter.state-manager/app/controller/create_states.py (6)
1-2: LGTM!Necessary import additions for the new functionality.
7-8: LGTM!Required model imports for graph template and node template handling.
23-23: LGTM!Function signature correctly updated to include the
graph_nameparameter.
28-31: LGTM!Proper graph template lookup with appropriate 404 error handling when the graph template is not found.
34-34: LGTM!Correct usage of the helper function to get the node template by identifier.
38-41: LGTM!State creation logic correctly uses the identifier from the input and derives node details from the matched template.
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: 1
🔭 Outside diff range comments (1)
state-manager/app/controller/create_states.py (1)
59-59: Missingidentifierfield in ResponseStateModelThe
ResponseStateModelnow requires anidentifierfield (added in line 15 of create_models.py), but it's not included in the response construction.- states=[ResponseStateModel(state_id=str(state.id), node_name=state.node_name, graph_name=state.graph_name, inputs=state.inputs, created_at=state.created_at) for state in newStates] + states=[ResponseStateModel(state_id=str(state.id), node_name=state.node_name, identifier=state.identifier, graph_name=state.graph_name, inputs=state.inputs, created_at=state.created_at) for state in newStates]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
state-manager/app/controller/create_states.py(1 hunks)state-manager/app/controller/enqueue_states.py(1 hunks)state-manager/app/models/create_models.py(1 hunks)state-manager/app/models/db/graph_template_model.py(2 hunks)state-manager/app/models/enqueue_response.py(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: NiveditJain
PR: exospherehost/exospherehost#132
File: state-manager/app/controller/upsert_graph_template.py:11-27
Timestamp: 2025-08-03T16:46:04.030Z
Learning: In the exospherehost codebase, for upsert operations on graph templates, the team prioritizes API idempotency over avoiding race conditions in the database layer implementation. The approach of separate find and insert/update operations is acceptable when the API behavior remains consistent.
📚 Learning: in the exospherehost codebase, for upsert operations on graph templates, the team prioritizes api id...
Learnt from: NiveditJain
PR: exospherehost/exospherehost#132
File: state-manager/app/controller/upsert_graph_template.py:11-27
Timestamp: 2025-08-03T16:46:04.030Z
Learning: In the exospherehost codebase, for upsert operations on graph templates, the team prioritizes API idempotency over avoiding race conditions in the database layer implementation. The approach of separate find and insert/update operations is acceptable when the API behavior remains consistent.
Applied to files:
state-manager/app/controller/create_states.py
🧬 Code Graph Analysis (3)
state-manager/app/models/create_models.py (1)
state-manager/app/models/graph_models.py (1)
UpsertGraphTemplateRequest(8-9)
state-manager/app/models/db/graph_template_model.py (1)
state-manager/app/models/node_template_model.py (1)
NodeTemplate(5-11)
state-manager/app/controller/create_states.py (4)
state-manager/app/models/state_status_enum.py (1)
StateStatusEnum(4-14)state-manager/app/models/db/state.py (1)
State(7-16)state-manager/app/models/db/graph_template_model.py (2)
GraphTemplate(13-86)get_node_by_identifier(35-40)state-manager/app/models/node_template_model.py (1)
NodeTemplate(5-11)
🔇 Additional comments (6)
state-manager/app/models/enqueue_response.py (1)
9-9: LGTM!The addition of the
identifierfield toStateModelis consistent with the broader refactoring to use node identifiers throughout the codebase.state-manager/app/controller/enqueue_states.py (1)
43-43: LGTM!Correctly includes the
identifierfield from the state object in the response model.state-manager/app/models/create_models.py (2)
8-8: LGTM!The
identifierfield with its accurate description properly replaces the previousnode_nameandgraph_namefields in the request model.
15-15: LGTM!The addition of the
identifierfield toResponseStateModelmaintains consistency with the identifier-based node referencing pattern.state-manager/app/controller/create_states.py (2)
16-21: LGTM!The helper function efficiently uses the new O(1) lookup method from GraphTemplate, addressing the previous performance concern.
28-34: LGTM!Proper validation of graph template existence and node template retrieval using the identifier.
No description provided.