Skip to content

Commit 3804274

Browse files
Enhance state management and logging functionality (#232)
* Enhance state management and logging functionality - Updated `serve` function in `run.py` to utilize the `workers` argument in development mode. - Improved `executed_state` function in `executed_state.py` to handle multiple new states more efficiently, including error handling for state insertion. - Modified `LogsManager` in `logs_manager.py` to dynamically set logging level based on the application mode (development or production). - Added logging for state creation in `create_next_state.py` to improve traceability. These changes aim to enhance the application's performance and maintainability in different environments. * Update state-manager/app/controller/executed_state.py Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * fixed tests * added more tests --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
1 parent 2326ef0 commit 3804274

8 files changed

Lines changed: 380 additions & 8 deletions

File tree

state-manager/app/controller/executed_state.py

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
from beanie import PydanticObjectId
2+
from beanie.operators import In
23
from app.models.executed_models import ExecutedRequestModel, ExecutedResponseModel
34

45
from fastapi import HTTPException, status, BackgroundTasks
@@ -36,11 +37,11 @@ async def executed_state(namespace_name: str, state_id: PydanticObjectId, body:
3637
state.parents = {**state.parents, state.identifier: state.id}
3738

3839
await state.save()
39-
4040
background_tasks.add_task(create_next_state, state)
4141

42+
new_states = []
4243
for output in body.outputs[1:]:
43-
new_state = State(
44+
new_states.append(State(
4445
node_name=state.node_name,
4546
namespace_name=state.namespace_name,
4647
identifier=state.identifier,
@@ -54,9 +55,20 @@ async def executed_state(namespace_name: str, state_id: PydanticObjectId, body:
5455
**state.parents,
5556
state.identifier: state.id
5657
}
57-
)
58-
await new_state.save()
59-
background_tasks.add_task(create_next_state, new_state)
58+
))
59+
60+
if len(new_states) > 0:
61+
inserted_ids = (await State.insert_many(new_states)).inserted_ids
62+
63+
inserted_states = await State.find(
64+
In(State.id, inserted_ids)
65+
).to_list()
66+
67+
if len(inserted_states) != len(new_states):
68+
raise RuntimeError(f"Failed to insert all new states. Expected {len(new_states)} states, but only {len(inserted_states)} were inserted")
69+
70+
for inserted_state in inserted_states:
71+
background_tasks.add_task(create_next_state, inserted_state)
6072

6173
return ExecutedResponseModel(status=StateStatusEnum.EXECUTED)
6274

state-manager/app/singletons/logs_manager.py

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
import structlog
22
import logging
3+
import os
4+
import sys
35
from .SingletonDecorator import singleton
46

57

@@ -28,9 +30,37 @@ def __init__(self):
2830
handler.setFormatter(formatter)
2931
logger = logging.getLogger()
3032
logger.addHandler(handler)
31-
logger.setLevel(logging.INFO)
33+
34+
# Check if running in development mode
35+
# Development mode is determined by the --mode argument passed to run.py
36+
is_development = self._is_development_mode()
37+
38+
if is_development:
39+
# In development mode, set level to WARNING to disable INFO logs
40+
logger.setLevel(logging.WARNING)
41+
else:
42+
# In production mode, keep INFO level
43+
logger.setLevel(logging.INFO)
3244

3345
self.logger = structlog.get_logger()
3446

47+
def _is_development_mode(self) -> bool:
48+
"""
49+
Check if the application is running in development mode.
50+
Development mode is determined by checking if '--mode' 'development'
51+
is in the command line arguments.
52+
"""
53+
# Check command line arguments for development mode
54+
if '--mode' in sys.argv:
55+
try:
56+
mode_index = sys.argv.index('--mode')
57+
if mode_index + 1 < len(sys.argv) and sys.argv[mode_index + 1] == 'development':
58+
return True
59+
except (ValueError, IndexError):
60+
pass
61+
62+
# Fallback: check environment variable
63+
return os.getenv('MODE', '').lower() == 'development'
64+
3565
def get_logger(self):
3666
return self.logger

state-manager/app/tasks/create_next_state.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
logger = LogsManager().get_logger()
1515

1616
async def create_next_state(state: State):
17+
logger.info(f"Creating next state for {state.identifier}")
1718
graph_template = None
1819

1920
if state is None or state.id is None:

state-manager/run.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,10 @@
1212

1313
def serve():
1414
mode = args.mode
15+
workers = args.workers
1516
if mode == "development":
16-
uvicorn.run("app.main:app", reload=True, host="0.0.0.0", port=8000)
17+
uvicorn.run("app.main:app", workers=workers, reload=True, host="0.0.0.0", port=8000)
1718
elif mode == "production":
18-
workers = args.workers
1919
print(f"Running with {workers} workers")
2020
uvicorn.run("app.main:app", workers=workers, host="0.0.0.0", port=8000)
2121
else:

state-manager/tests/unit/controller/test_executed_state.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,9 @@ async def test_executed_state_success_multiple_outputs(
118118
# Additional calls in the loop also return query objects with set method
119119
mock_state_class.find_one = AsyncMock(return_value=mock_state)
120120
mock_state.save = AsyncMock()
121+
new_ids = [PydanticObjectId(), PydanticObjectId()]
122+
mock_state_class.insert_many = AsyncMock(return_value=MagicMock(inserted_ids=new_ids))
123+
mock_state_class.find = MagicMock(return_value=AsyncMock(to_list=AsyncMock(return_value=[mock_state, mock_state])))
121124

122125
# Mock State.save() for new states
123126
mock_new_state = MagicMock()
@@ -264,3 +267,33 @@ async def test_executed_state_database_error(
264267

265268
assert str(exc_info.value) == "Database error"
266269

270+
@patch('app.controller.executed_state.State')
271+
@patch('app.controller.executed_state.create_next_state')
272+
async def test_executed_state_general_exception_handling(
273+
self,
274+
mock_create_next_state,
275+
mock_state_class,
276+
mock_namespace,
277+
mock_state_id,
278+
mock_executed_request,
279+
mock_state,
280+
mock_background_tasks,
281+
mock_request_id
282+
):
283+
"""Test general exception handling in executed_state function"""
284+
# Arrange
285+
mock_state_class.find_one = AsyncMock(return_value=mock_state)
286+
mock_state.save = AsyncMock(side_effect=Exception("Save error"))
287+
288+
# Act & Assert
289+
with pytest.raises(Exception) as exc_info:
290+
await executed_state(
291+
mock_namespace,
292+
mock_state_id,
293+
mock_executed_request,
294+
mock_request_id,
295+
mock_background_tasks
296+
)
297+
298+
assert str(exc_info.value) == "Save error"
299+
Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
import pytest
2+
from unittest.mock import AsyncMock, MagicMock, patch
3+
from beanie import PydanticObjectId
4+
5+
from app.tasks.create_next_state import create_next_state
6+
from app.models.db.state import State
7+
from app.models.db.graph_template_model import GraphTemplate
8+
from app.models.db.registered_node import RegisteredNode
9+
from app.models.graph_template_validation_status import GraphTemplateValidationStatus
10+
from app.models.state_status_enum import StateStatusEnum
11+
12+
13+
class TestCreateNextState:
14+
"""Test cases for create_next_state function"""
15+
16+
@pytest.fixture
17+
def mock_state(self):
18+
"""Create a mock state object"""
19+
state = MagicMock(spec=State)
20+
state.id = PydanticObjectId()
21+
state.identifier = "test_node"
22+
state.namespace_name = "test_namespace"
23+
state.graph_name = "test_graph"
24+
state.run_id = "test_run_id"
25+
state.status = StateStatusEnum.EXECUTED
26+
state.inputs = {"input1": "value1"}
27+
state.outputs = {"output1": "result1"}
28+
state.error = None
29+
state.parents = {"parent_node": PydanticObjectId()}
30+
state.save = AsyncMock()
31+
return state
32+
33+
@pytest.fixture
34+
def mock_graph_template(self):
35+
"""Create a mock graph template"""
36+
template = MagicMock(spec=GraphTemplate)
37+
template.validation_status = GraphTemplateValidationStatus.VALID
38+
template.get_node_by_identifier = MagicMock()
39+
return template
40+
41+
@pytest.fixture
42+
def mock_registered_node(self):
43+
"""Create a mock registered node"""
44+
node = MagicMock(spec=RegisteredNode)
45+
node.inputs_schema = {
46+
"type": "object",
47+
"properties": {
48+
"field1": {"type": "string"},
49+
"field2": {"type": "string"}
50+
}
51+
}
52+
return node
53+
54+
@patch('app.tasks.create_next_state.GraphTemplate')
55+
async def test_create_next_state_none_id(self, mock_graph_template_class):
56+
"""Test create_next_state with state having None id"""
57+
# Arrange
58+
state_with_none_id = MagicMock()
59+
state_with_none_id.id = None
60+
61+
# Act & Assert
62+
with pytest.raises(ValueError, match="State is not valid"):
63+
await create_next_state(state_with_none_id)
64+
65+
@patch('app.tasks.create_next_state.GraphTemplate')
66+
@patch('app.tasks.create_next_state.asyncio.sleep')
67+
async def test_create_next_state_wait_for_validation(
68+
self,
69+
mock_sleep,
70+
mock_graph_template_class,
71+
mock_state,
72+
mock_graph_template
73+
):
74+
"""Test waiting for graph template to become valid"""
75+
# Arrange
76+
# First call returns invalid template, second call returns valid
77+
invalid_template = MagicMock()
78+
invalid_template.validation_status = GraphTemplateValidationStatus.INVALID
79+
80+
mock_graph_template_class.find_one = AsyncMock(side_effect=[invalid_template, mock_graph_template])
81+
82+
# Mock node template with no next nodes
83+
node_template = MagicMock()
84+
node_template.next_nodes = None
85+
mock_graph_template.get_node_by_identifier.return_value = node_template
86+
87+
# Act
88+
await create_next_state(mock_state)
89+
90+
# Assert
91+
assert mock_graph_template_class.find_one.call_count == 2
92+
mock_sleep.assert_called_once_with(1)
93+
assert mock_state.status == StateStatusEnum.SUCCESS
94+
95+
@patch('app.tasks.create_next_state.GraphTemplate')
96+
async def test_create_next_state_no_next_nodes(
97+
self,
98+
mock_graph_template_class,
99+
mock_state,
100+
mock_graph_template
101+
):
102+
"""Test when there are no next nodes"""
103+
# Arrange
104+
mock_graph_template_class.find_one = AsyncMock(return_value=mock_graph_template)
105+
106+
node_template = MagicMock()
107+
node_template.next_nodes = None
108+
mock_graph_template.get_node_by_identifier.return_value = node_template
109+
110+
# Act
111+
await create_next_state(mock_state)
112+
113+
# Assert
114+
assert mock_state.status == StateStatusEnum.SUCCESS
115+
116+
@patch('app.tasks.create_next_state.GraphTemplate')
117+
async def test_create_next_state_general_exception(
118+
self,
119+
mock_graph_template_class,
120+
mock_state
121+
):
122+
"""Test general exception handling"""
123+
# Arrange
124+
mock_graph_template_class.find_one = AsyncMock(side_effect=Exception("General error"))
125+
126+
# Act
127+
await create_next_state(mock_state)
128+
129+
# Assert
130+
assert mock_state.status == StateStatusEnum.ERRORED
131+
assert mock_state.error == "General error"
132+
mock_state.save.assert_called_once()
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
import logging
2+
from unittest.mock import patch
3+
from app.singletons.logs_manager import LogsManager
4+
5+
6+
class TestLogsManager:
7+
"""Test cases for LogsManager singleton"""
8+
9+
def setup_method(self):
10+
"""Reset the singleton and logging before each test"""
11+
# Clear the singleton instance
12+
if hasattr(LogsManager, '_instance'):
13+
delattr(LogsManager, '_instance')
14+
15+
# Reset logging level to INFO
16+
logging.getLogger().setLevel(logging.INFO)
17+
18+
def teardown_method(self):
19+
"""Clean up after each test"""
20+
# Clear the singleton instance
21+
if hasattr(LogsManager, '_instance'):
22+
delattr(LogsManager, '_instance')
23+
24+
@patch('app.singletons.logs_manager.sys.argv', ['python', 'run.py', '--mode', 'production'])
25+
def test_logs_manager_production_mode_command_line(self):
26+
"""Test LogsManager sets INFO level in production mode via command line"""
27+
# Check that the logging level is set to INFO in production mode
28+
root_logger = logging.getLogger()
29+
assert root_logger.level == logging.INFO
30+
31+
@patch('app.singletons.logs_manager.sys.argv', ['python', 'run.py', '--mode'])
32+
def test_logs_manager_invalid_command_line_format(self):
33+
"""Test LogsManager handles invalid command line format gracefully"""
34+
# Should default to INFO level when command line format is invalid
35+
root_logger = logging.getLogger()
36+
assert root_logger.level == logging.INFO
37+
38+
@patch('app.singletons.logs_manager.sys.argv', ['python', 'run.py', '--mode', 'invalid'])
39+
def test_logs_manager_invalid_mode_command_line(self):
40+
"""Test LogsManager handles invalid mode in command line"""
41+
# Should default to INFO level when mode is invalid
42+
root_logger = logging.getLogger()
43+
assert root_logger.level == logging.INFO
44+
45+
def test_logs_manager_singleton_pattern(self):
46+
"""Test LogsManager follows singleton pattern"""
47+
logs_manager1 = LogsManager()
48+
logs_manager2 = LogsManager()
49+
50+
# Both instances should be the same object
51+
assert logs_manager1 is logs_manager2
52+
53+
def test_get_logger_returns_structlog_logger(self):
54+
"""Test get_logger returns a structlog logger"""
55+
logs_manager = LogsManager()
56+
logger = logs_manager.get_logger()
57+
58+
# Should return a structlog logger
59+
assert logger is not None
60+
# Check that it's a structlog logger by checking for structlog-specific attributes
61+
assert hasattr(logger, 'info')
62+
assert hasattr(logger, 'error')
63+
assert hasattr(logger, 'warning')

0 commit comments

Comments
 (0)