-
Notifications
You must be signed in to change notification settings - Fork 42
Unit & Integration tests for State Manager #200
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
Changes from all commits
8d7988a
7c3b0b4
327fd6d
1c58eb8
2ae95e3
586a6b6
6d7d76f
bb9e3f1
104f58a
009818f
f23a7fa
ffd5bf0
45d6428
2d3f7fb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| name: State Manager Unit Tests | ||
|
|
||
| on: | ||
| push: | ||
| branches: [main] | ||
| paths: | ||
| - 'state-manager/**' | ||
| pull_request: | ||
| branches: [main] | ||
| paths: | ||
| - 'state-manager/**' | ||
|
|
||
| jobs: | ||
| test: | ||
| runs-on: ubuntu-latest | ||
| services: | ||
| mongodb: | ||
| image: mongo:7 | ||
| ports: | ||
| - 27017:27017 | ||
| options: >- | ||
| --health-cmd "mongosh --eval 'db.runCommand(\"ping\")'" | ||
| --health-interval 10s | ||
| --health-timeout 5s | ||
| --health-retries 5 | ||
|
|
||
| steps: | ||
| - name: Checkout code | ||
| uses: actions/checkout@v4 | ||
|
|
||
| - name: Set up Python | ||
| uses: actions/setup-python@v5 | ||
| with: | ||
| python-version: '3.12' | ||
|
|
||
| - name: Install uv | ||
| uses: astral-sh/setup-uv@v2 | ||
| with: | ||
| cache: true | ||
|
|
||
| - name: Install dev dependencies with uv | ||
| working-directory: state-manager | ||
| run: | | ||
| uv sync --group dev | ||
|
|
||
| - name: Run unit tests with pytest and coverage | ||
| working-directory: state-manager | ||
| run: | | ||
| uv run pytest tests/unit/ --cov=app --cov-report=xml --cov-report=term-missing -v --junitxml=pytest-report.xml | ||
|
|
||
| - name: Upload coverage reports to Codecov | ||
| uses: codecov/codecov-action@v5 | ||
| with: | ||
| token: ${{ secrets.CODECOV_TOKEN }} | ||
| slug: exospherehost/exospherehost | ||
| files: state-manager/coverage.xml | ||
| flags: state-manager-unittests | ||
| name: state-manager-coverage-report | ||
| fail_ci_if_error: true | ||
|
|
||
| - name: Upload test results | ||
| uses: actions/upload-artifact@v4 | ||
| if: always() | ||
| with: | ||
| name: state-manager-test-results | ||
| path: state-manager/pytest-report.xml | ||
| retention-days: 30 |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,14 +1,15 @@ | ||||||||||||||||||||||||||||||||||||
| from fastapi import HTTPException | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| from app.singletons.logs_manager import LogsManager | ||||||||||||||||||||||||||||||||||||
| from app.models.create_models import CreateRequestModel, CreateResponseModel, ResponseStateModel | ||||||||||||||||||||||||||||||||||||
| from app.models.create_models import CreateRequestModel, CreateResponseModel, ResponseStateModel, TriggerGraphRequestModel, TriggerGraphResponseModel | ||||||||||||||||||||||||||||||||||||
| from app.models.state_status_enum import StateStatusEnum | ||||||||||||||||||||||||||||||||||||
| from app.models.db.state import State | ||||||||||||||||||||||||||||||||||||
| from app.models.db.graph_template_model import GraphTemplate | ||||||||||||||||||||||||||||||||||||
| from app.models.node_template_model import NodeTemplate | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| from beanie.operators import In | ||||||||||||||||||||||||||||||||||||
| from bson import ObjectId | ||||||||||||||||||||||||||||||||||||
| from beanie import PydanticObjectId | ||||||||||||||||||||||||||||||||||||
| import uuid | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| logger = LogsManager().get_logger() | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
|
|
@@ -20,6 +21,33 @@ def get_node_template(graph_template: GraphTemplate, identifier: str) -> NodeTem | |||||||||||||||||||||||||||||||||||
| return node | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| async def trigger_graph(namespace_name: str, graph_name: str, body: TriggerGraphRequestModel, x_exosphere_request_id: str) -> TriggerGraphResponseModel: | ||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||
| # Generate a new run ID for this graph execution | ||||||||||||||||||||||||||||||||||||
| run_id = str(uuid.uuid4()) | ||||||||||||||||||||||||||||||||||||
| logger.info(f"Triggering graph {graph_name} with run_id {run_id}", x_exosphere_request_id=x_exosphere_request_id) | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| # Create a CreateRequestModel with the generated run_id | ||||||||||||||||||||||||||||||||||||
| create_request = CreateRequestModel( | ||||||||||||||||||||||||||||||||||||
| run_id=run_id, | ||||||||||||||||||||||||||||||||||||
| states=body.states | ||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| # Call the existing create_states function | ||||||||||||||||||||||||||||||||||||
| create_response = await create_states(namespace_name, graph_name, create_request, x_exosphere_request_id) | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| # Return the trigger response with the generated run_id | ||||||||||||||||||||||||||||||||||||
| return TriggerGraphResponseModel( | ||||||||||||||||||||||||||||||||||||
| run_id=run_id, | ||||||||||||||||||||||||||||||||||||
| status=create_response.status, | ||||||||||||||||||||||||||||||||||||
| states=create_response.states | ||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| except Exception as e: | ||||||||||||||||||||||||||||||||||||
| logger.error(f"Error triggering graph {graph_name} for namespace {namespace_name}", x_exosphere_request_id=x_exosphere_request_id) | ||||||||||||||||||||||||||||||||||||
| raise e | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| async def create_states(namespace_name: str, graph_name: str, body: CreateRequestModel, x_exosphere_request_id: str) -> CreateResponseModel: | ||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||
| states = [] | ||||||||||||||||||||||||||||||||||||
|
|
@@ -39,6 +67,7 @@ async def create_states(namespace_name: str, graph_name: str, body: CreateReques | |||||||||||||||||||||||||||||||||||
| node_name=node_template.node_name, | ||||||||||||||||||||||||||||||||||||
| namespace_name=node_template.namespace, | ||||||||||||||||||||||||||||||||||||
| graph_name=graph_name, | ||||||||||||||||||||||||||||||||||||
| run_id=body.run_id, | ||||||||||||||||||||||||||||||||||||
| status=StateStatusEnum.CREATED, | ||||||||||||||||||||||||||||||||||||
| inputs=state.inputs, | ||||||||||||||||||||||||||||||||||||
| outputs={}, | ||||||||||||||||||||||||||||||||||||
|
|
@@ -51,12 +80,12 @@ async def create_states(namespace_name: str, graph_name: str, body: CreateReques | |||||||||||||||||||||||||||||||||||
| logger.info(f"Created states: {inserted_states.inserted_ids}", x_exosphere_request_id=x_exosphere_request_id) | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| newStates = await State.find( | ||||||||||||||||||||||||||||||||||||
| In(State.id, [ObjectId(id) for id in inserted_states.inserted_ids]) | ||||||||||||||||||||||||||||||||||||
| In(State.id, [PydanticObjectId(id) for id in inserted_states.inserted_ids]) | ||||||||||||||||||||||||||||||||||||
| ).to_list() | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| return CreateResponseModel( | ||||||||||||||||||||||||||||||||||||
| status=StateStatusEnum.CREATED, | ||||||||||||||||||||||||||||||||||||
| states=[ResponseStateModel(state_id=str(state.id), identifier=state.identifier, 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), identifier=state.identifier, node_name=state.node_name, graph_name=state.graph_name, run_id=state.run_id, inputs=state.inputs, created_at=state.created_at) for state in newStates] | ||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
86
to
89
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Maintain variable naming consistency (snake_case) Use new_states in the response builder. - return CreateResponseModel(
- status=StateStatusEnum.CREATED,
- states=[ResponseStateModel(state_id=str(state.id), identifier=state.identifier, node_name=state.node_name, graph_name=state.graph_name, run_id=state.run_id, inputs=state.inputs, created_at=state.created_at) for state in newStates]
- )
+ return CreateResponseModel(
+ status=StateStatusEnum.CREATED,
+ states=[ResponseStateModel(
+ state_id=str(state.id),
+ identifier=state.identifier,
+ node_name=state.node_name,
+ graph_name=state.graph_name,
+ run_id=state.run_id,
+ inputs=state.inputs,
+ created_at=state.created_at
+ ) for state in new_states]
+ )📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| except Exception as e: | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,5 +1,6 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| from beanie import PydanticObjectId | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| from app.models.executed_models import ExecutedRequestModel, ExecutedResponseModel | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| from bson import ObjectId | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| from fastapi import HTTPException, status, BackgroundTasks | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| from app.models.db.state import State | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -9,7 +10,7 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger = LogsManager().get_logger() | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| async def executed_state(namespace_name: str, state_id: ObjectId, body: ExecutedRequestModel, x_exosphere_request_id: str, background_tasks: BackgroundTasks) -> ExecutedResponseModel: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| async def executed_state(namespace_name: str, state_id: PydanticObjectId, body: ExecutedRequestModel, x_exosphere_request_id: str, background_tasks: BackgroundTasks) -> ExecutedResponseModel: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.info(f"Executed state {state_id} for namespace {namespace_name}", x_exosphere_request_id=x_exosphere_request_id) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -22,17 +23,18 @@ async def executed_state(namespace_name: str, state_id: ObjectId, body: Executed | |||||||||||||||||||||||||||||||||||||||||||||||||||
| raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail="State is not queued") | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| if len(body.outputs) == 0: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| await State.find_one(State.id == state_id).set( | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| {"status": StateStatusEnum.EXECUTED, "outputs": {}, "parents": {**state.parents, state.identifier: state.id}} | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| state.status = StateStatusEnum.EXECUTED | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| state.outputs = {} | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| state.parents = {**state.parents, state.identifier: state.id} | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| await state.save() | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| background_tasks.add_task(create_next_state, state) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
25
to
+33
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Prefer truthy check for outputs and reduce branching duplication Use a truthy check for readability and consider factoring the common mutation code (parents update + save + scheduling) to avoid duplication. - if len(body.outputs) == 0:
+ if not body.outputs:
state.status = StateStatusEnum.EXECUTED
state.outputs = {}
state.parents = {**state.parents, state.identifier: state.id}
await state.save()
background_tasks.add_task(create_next_state, state)
- else:
+ else:
state.outputs = body.outputs[0]
state.status = StateStatusEnum.EXECUTED
state.parents = {**state.parents, state.identifier: state.id}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||
| state.outputs = body.outputs[0] | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| state.status = StateStatusEnum.EXECUTED | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| state.parents = {**state.parents, state.identifier: state.id} | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| await state.save() | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| background_tasks.add_task(create_next_state, state) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -43,6 +45,7 @@ async def executed_state(namespace_name: str, state_id: ObjectId, body: Executed | |||||||||||||||||||||||||||||||||||||||||||||||||||
| namespace_name=state.namespace_name, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| identifier=state.identifier, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| graph_name=state.graph_name, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| run_id=state.run_id, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| status=StateStatusEnum.CREATED, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| inputs=state.inputs, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| outputs=output, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,37 @@ | ||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||
| Controller for fetching current states in a namespace | ||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||
| from typing import List | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| from ..models.db.state import State | ||||||||||||||||||||||||||||||||||||||||||||||
| from ..singletons.logs_manager import LogsManager | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| async def get_current_states(namespace: str, request_id: str) -> List[State]: | ||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||
| Get all current states in a namespace | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| Args: | ||||||||||||||||||||||||||||||||||||||||||||||
| namespace: The namespace to search in | ||||||||||||||||||||||||||||||||||||||||||||||
| request_id: Request ID for logging | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| Returns: | ||||||||||||||||||||||||||||||||||||||||||||||
| List of all states in the namespace | ||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+10
to
+20
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Align parameter names with other controllers (namespace_name, x_exosphere_request_id) Other controllers use namespace_name and x_exosphere_request_id. Rename for consistency and easier grepping across the codebase. -async def get_current_states(namespace: str, request_id: str) -> List[State]:
+async def get_current_states(namespace_name: str, x_exosphere_request_id: str) -> List[State]:
@@
- namespace: The namespace to search in
- request_id: Request ID for logging
+ namespace_name: The namespace to search in
+ x_exosphere_request_id: Request ID for logging📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||
| logger = LogsManager().get_logger() | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||||||||||||
| logger.info(f"Fetching current states for namespace: {namespace}", x_exosphere_request_id=request_id) | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| # Find all states in the namespace | ||||||||||||||||||||||||||||||||||||||||||||||
| states = await State.find( | ||||||||||||||||||||||||||||||||||||||||||||||
| State.namespace_name == namespace | ||||||||||||||||||||||||||||||||||||||||||||||
| ).to_list() | ||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+27
to
+29
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Potentially unbounded query — consider pagination/limits This will load all states for a namespace into memory. For large namespaces, this can be slow and memory-heavy. Consider adding optional limit/offset (or a created_after cursor) and a deterministic sort (e.g., by created_at). Example API shape (outside this hunk):
💡 Verification agent 🧩 Analysis chainEnsure there’s an index on State.namespace_name Filtering by namespace_name without an index can cause full scans. Verify an index exists on this field (via Beanie Indexed or model Settings.indexes). If missing, add one. If no index is present, consider (in State model):
Length of output: 1034 Add a database index on We didn’t find any existing indexes on • File: state-manager/app/models/db/state.py Suggested changes (choose one): Option A – Field-level index - namespace_name: str = Field(..., description="Name of the namespace of the state")
+ namespace_name: str = Field(..., index=True, description="Name of the namespace of the state")Option B – Model-level index +from beanie import IndexModel
+from pymongo import ASCENDING
class State(BaseDatabaseModel):
node_name: str = Field(..., description="Name of the node of the state")
namespace_name: str = Field(..., description="Name of the namespace of the state")
identifier: str = Field(..., description="Identifier of the node for which state is created")
+ class Settings:
+ indexes = [
+ IndexModel([("namespace_name", ASCENDING)], name="idx_state_namespace")
+ ]Ensure you import 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| logger.info(f"Found {len(states)} states for namespace: {namespace}", x_exosphere_request_id=request_id) | ||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+24
to
+31
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Use structured logging for info logs (avoid f-strings) and align field names Pass contextual fields as structured args to align with other controllers and improve log parsing/correlation. Also update variable names per the previous comment. - logger.info(f"Fetching current states for namespace: {namespace}", x_exosphere_request_id=request_id)
+ logger.info(
+ "Fetching current states for namespace",
+ namespace=namespace_name,
+ x_exosphere_request_id=x_exosphere_request_id,
+ )
@@
- states = await State.find(
- State.namespace_name == namespace
- ).to_list()
+ states = await State.find(
+ State.namespace_name == namespace_name
+ ).to_list()
@@
- logger.info(f"Found {len(states)} states for namespace: {namespace}", x_exosphere_request_id=request_id)
+ logger.info(
+ "Found states for namespace",
+ namespace=namespace_name,
+ count=len(states),
+ x_exosphere_request_id=x_exosphere_request_id,
+ )
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| return states | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| except Exception as e: | ||||||||||||||||||||||||||||||||||||||||||||||
| logger.error(f"Error fetching current states for namespace {namespace}: {str(e)}", x_exosphere_request_id=request_id) | ||||||||||||||||||||||||||||||||||||||||||||||
| raise | ||||||||||||||||||||||||||||||||||||||||||||||
nk-ag marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,39 @@ | ||||||||||||||||||
| """ | ||||||||||||||||||
| Controller for fetching states by run ID | ||||||||||||||||||
| """ | ||||||||||||||||||
| from typing import List | ||||||||||||||||||
|
|
||||||||||||||||||
| from ..models.db.state import State | ||||||||||||||||||
| from ..singletons.logs_manager import LogsManager | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
||||||||||||||||||
| async def get_states_by_run_id(namespace: str, run_id: str, request_id: str) -> List[State]: | ||||||||||||||||||
| """ | ||||||||||||||||||
| Get all states for a given run ID in a namespace | ||||||||||||||||||
|
|
||||||||||||||||||
| Args: | ||||||||||||||||||
| namespace: The namespace to search in | ||||||||||||||||||
| run_id: The run ID to filter by | ||||||||||||||||||
| request_id: Request ID for logging | ||||||||||||||||||
|
|
||||||||||||||||||
| Returns: | ||||||||||||||||||
| List of states for the given run ID | ||||||||||||||||||
| """ | ||||||||||||||||||
|
Comment on lines
+10
to
+21
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Align parameter naming with the rest of controllers ( Other controllers consistently use Apply: -async def get_states_by_run_id(namespace: str, run_id: str, request_id: str) -> List[State]:
+async def get_states_by_run_id(namespace_name: str, run_id: str, request_id: str) -> List[State]:
@@
- Get all states for a given run ID in a namespace
+ Get all states for a given run ID in a namespace
@@
- namespace: The namespace to search in
+ namespace_name: The namespace to search in
@@
- logger.info(f"Fetching states for run ID: {run_id} in namespace: {namespace}", x_exosphere_request_id=request_id)
+ logger.info(f"Fetching states for run ID: {run_id} in namespace: {namespace_name}", x_exosphere_request_id=request_id)And below in the query, replace
🤖 Prompt for AI Agents |
||||||||||||||||||
| logger = LogsManager().get_logger() | ||||||||||||||||||
|
|
||||||||||||||||||
| try: | ||||||||||||||||||
| logger.info(f"Fetching states for run ID: {run_id} in namespace: {namespace}", x_exosphere_request_id=request_id) | ||||||||||||||||||
|
|
||||||||||||||||||
| # Find all states for the run ID in the namespace | ||||||||||||||||||
| states = await State.find( | ||||||||||||||||||
| State.run_id == run_id, | ||||||||||||||||||
| State.namespace_name == namespace | ||||||||||||||||||
| ).to_list() | ||||||||||||||||||
|
Comment on lines
+28
to
+31
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Consider indexing and deterministic ordering
🤖 Prompt for AI Agents |
||||||||||||||||||
|
|
||||||||||||||||||
| logger.info(f"Found {len(states)} states for run ID: {run_id}", x_exosphere_request_id=request_id) | ||||||||||||||||||
|
|
||||||||||||||||||
| return states | ||||||||||||||||||
|
|
||||||||||||||||||
| except Exception as e: | ||||||||||||||||||
| logger.error(f"Error fetching states for run ID {run_id} in namespace {namespace}: {str(e)}", x_exosphere_request_id=request_id) | ||||||||||||||||||
| raise | ||||||||||||||||||
|
Comment on lines
+38
to
+39
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Log structured error like other controllers for consistency Keep logging style consistent across controllers. - logger.error(f"Error fetching states for run ID {run_id} in namespace {namespace}: {str(e)}", x_exosphere_request_id=request_id)
+ logger.error(f"Error fetching states for run ID {run_id} in namespace {namespace}", x_exosphere_request_id=request_id, error=e)📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,37 @@ | ||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||
| Controller for listing graph templates by namespace | ||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||
| from typing import List | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| from ..models.db.graph_template_model import GraphTemplate | ||||||||||||||||||||||||||
| from ..singletons.logs_manager import LogsManager | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| async def list_graph_templates(namespace: str, request_id: str) -> List[GraphTemplate]: | ||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||
| List all graph templates for a given namespace | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Args: | ||||||||||||||||||||||||||
| namespace: The namespace to list graph templates for | ||||||||||||||||||||||||||
| request_id: Request ID for logging | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Returns: | ||||||||||||||||||||||||||
| List of graph templates | ||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||
| logger = LogsManager().get_logger() | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||
| logger.info(f"Listing graph templates for namespace: {namespace}", x_exosphere_request_id=request_id) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| # Find all graph templates for the namespace | ||||||||||||||||||||||||||
| templates = await GraphTemplate.find( | ||||||||||||||||||||||||||
| GraphTemplate.namespace == namespace | ||||||||||||||||||||||||||
| ).to_list() | ||||||||||||||||||||||||||
|
Comment on lines
+27
to
+29
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainEnsure DB index on namespace filter This query will be frequent; ensure GraphTemplate.namespace is indexed to keep the list operation efficient. 🏁 Script executed: #!/bin/bash
set -e
# Locate the GraphTemplate class definition
rg -n "class GraphTemplate" .Length of output: 250 🏁 Script executed: #!/bin/bash
set -e
# Search for namespace field usage and index definitions in GraphTemplate model
rg -n "namespace" state-manager/app/models/db/graph_template_model.py
rg -n "__table_args__" state-manager/app/models/db/graph_template_model.py
rg -n "indexes" state-manager/app/models/db/graph_template_model.py
rg -n "Index" state-manager/app/models/db/graph_template_model.py
# Preview the beginning of the model file to inspect field and meta definitions
sed -n '1,200p' state-manager/app/models/db/graph_template_model.pyLength of output: 331 Add single‐field index on • File: Example diff: class GraphTemplate(BaseDatabaseModel):
...
__table_args__ = (
- Index("name", "namespace", unique=True, name="unique_name_namespace"),
+ Index("name", "namespace", unique=True, name="unique_name_namespace"),
+ Index("namespace", name="idx_graph_template_namespace"),
)📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| logger.info(f"Found {len(templates)} graph templates for namespace: {namespace}", x_exosphere_request_id=request_id) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| return templates | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
Comment on lines
+27
to
+34
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Optional: sort or paginate results For consistency and predictability, consider returning templates sorted (e.g., by name) and/or add pagination to avoid large result sets. Happy to provide a patch introducing pagination in the controller and response model. 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||
| except Exception as e: | ||||||||||||||||||||||||||
| logger.error(f"Error listing graph templates for namespace {namespace}: {str(e)}", x_exosphere_request_id=request_id) | ||||||||||||||||||||||||||
| raise | ||||||||||||||||||||||||||
|
Comment on lines
+35
to
+37
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Improve error logging (structured + stack trace) Align with logging style used elsewhere: include structured fields and exc_info=True. Apply this diff: - logger.error(f"Error listing graph templates for namespace {namespace}: {str(e)}", x_exosphere_request_id=request_id)
+ logger.error(
+ "Error listing graph templates",
+ namespace=namespace,
+ x_exosphere_request_id=request_id,
+ error=e,
+ exc_info=True,
+ )📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,37 @@ | ||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||
| Controller for listing registered nodes by namespace | ||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||
| from typing import List | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| from ..models.db.registered_node import RegisteredNode | ||||||||||||||||||||||||||
| from ..singletons.logs_manager import LogsManager | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| async def list_registered_nodes(namespace: str, request_id: str) -> List[RegisteredNode]: | ||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||
| List all registered nodes for a given namespace | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Args: | ||||||||||||||||||||||||||
| namespace: The namespace to list nodes for | ||||||||||||||||||||||||||
| request_id: Request ID for logging | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Returns: | ||||||||||||||||||||||||||
| List of registered nodes | ||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||
| logger = LogsManager().get_logger() | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||
| logger.info(f"Listing registered nodes for namespace: {namespace}", x_exosphere_request_id=request_id) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| # Find all registered nodes for the namespace | ||||||||||||||||||||||||||
| nodes = await RegisteredNode.find( | ||||||||||||||||||||||||||
| RegisteredNode.namespace == namespace | ||||||||||||||||||||||||||
| ).to_list() | ||||||||||||||||||||||||||
|
Comment on lines
+27
to
+29
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainEnsure DB index on namespace Add or verify an index on RegisteredNode.namespace to keep list queries fast at scale. 🏁 Script executed: #!/bin/bash
set -e
echo "Locating RegisteredNode model definition"
rg -n "class RegisteredNode" -C5 || true
echo "Checking for index configuration (e.g., `indexes` attribute)"
rg -n "indexes" -C5 -g "*.py" || true
echo "Searching for `IndexModel` usage"
rg -n "IndexModel" -C5 -g "*.py" || trueLength of output: 5822 🏁 Script executed: #!/bin/bash
set -e
echo "Locating BaseDatabaseModel definition"
rg -n "class BaseDatabaseModel" -C5 -g "state-manager/app/models/db/base.py" || true
echo "Printing BaseDatabaseModel contents"
sed -n '1,200p' state-manager/app/models/db/base.py || trueLength of output: 1657 Add a DB index on The • state-manager/app/models/db/registered_node.py from pymongo import IndexModel
# … other imports …
class RegisteredNode(BaseDatabaseModel):
name: str = Field(..., description="Unique name of the registered node")
namespace: str = Field(..., description="Namespace of the registered node")
# … other fields …
class Settings:
indexes = [
IndexModel(
keys=[("namespace", 1)],
name="idx_registerednode_namespace"
)
]Also ensure that this index is applied to the existing collection (via your migration or startup logic). 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| logger.info(f"Found {len(nodes)} registered nodes for namespace: {namespace}", x_exosphere_request_id=request_id) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| return nodes | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
Comment on lines
+27
to
+34
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Consider adding pagination/sorting to avoid large payloads Listing all nodes unbounded can become costly. Consider paginating and/or sorting (e.g., by name) to make responses predictable and scalable. If you want, I can draft a paginated query pattern using Beanie and extend the response model accordingly. 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||
| except Exception as e: | ||||||||||||||||||||||||||
| logger.error(f"Error listing registered nodes for namespace {namespace}: {str(e)}", x_exosphere_request_id=request_id) | ||||||||||||||||||||||||||
| raise | ||||||||||||||||||||||||||
|
Comment on lines
+35
to
+37
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Log exceptions with structured fields and stack trace Prefer structured error logging with the exception object and stack trace for observability parity with other controllers (e.g., get_graph_template). Apply this diff: - logger.error(f"Error listing registered nodes for namespace {namespace}: {str(e)}", x_exosphere_request_id=request_id)
+ logger.error(
+ "Error listing registered nodes",
+ namespace=namespace,
+ x_exosphere_request_id=request_id,
+ error=e,
+ exc_info=True,
+ )📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.