SIENTIAPDE-1253: Refactor training workflow and activities to raise exceptions on failure
This commit refactors the training workflow and associated activities to raise exceptions on failure instead of returning success/failure dictionaries. This allows the Temporal workflow to handle errors more effectively and ensures that the workflow stops when a critical error occurs. Key changes: - The train_model workflow is introduced to orchestrate the entire training process, including parameter validation, data download, model training, and model saving. - The validate_train_params activity is added to validate and convert training parameters. - The train_model and save_model activities are updated to raise exceptions on failure. - The ExperimentStatus enum is updated to include a new status for orchestrator validation errors. - The tests are updated to reflect the new exception-based error handling. - The activities now return the TrainModelResult directly instead of a dictionary.
This commit is contained in:
@@ -1,6 +1,7 @@
|
||||
from unittest.mock import ANY, MagicMock, patch
|
||||
|
||||
import numpy as np
|
||||
import pytest
|
||||
from pytest import fixture, mark
|
||||
from sientia_do.notifications.models import NotificationLevel
|
||||
from sientia_do.temporal.constants import DATETIME_FORMAT, DATETIME_FORMAT_WITH_TZ
|
||||
@@ -333,10 +334,9 @@ async def test_save_model_success(mlflow):
|
||||
mlflow.model_monitoring_repository.generate_artifacts.assert_called_once_with(train_result)
|
||||
mlflow.model_monitoring_repository.save_run.assert_called_once_with(train_result)
|
||||
|
||||
# Verify response
|
||||
assert response['success'] is True
|
||||
assert response['result'] == train_result
|
||||
assert response['error_message'] is None
|
||||
# Verify response - now returns TrainModelResult directly
|
||||
assert response == train_result
|
||||
assert response.run_name == 'test_experiment-1'
|
||||
assert train_result.run_name == 'test_experiment-1'
|
||||
|
||||
|
||||
@@ -362,13 +362,9 @@ async def test_save_model_get_next_run_name_error(mlflow):
|
||||
'train_result': train_result,
|
||||
}
|
||||
|
||||
# Call the method
|
||||
response = await mlflow.save_model(input_data)
|
||||
|
||||
# Verify error handling
|
||||
assert response['success'] is False
|
||||
assert response['result'] is None
|
||||
assert 'MLflow connection error' in response['error_message']
|
||||
# Call the method - should raise exception
|
||||
with pytest.raises(Exception, match='MLflow connection error'):
|
||||
await mlflow.save_model(input_data)
|
||||
|
||||
# Verify notification was sent
|
||||
mlflow.send_notification.assert_called_once_with(
|
||||
@@ -404,13 +400,9 @@ async def test_save_model_generate_artifacts_error(mlflow):
|
||||
'train_result': train_result,
|
||||
}
|
||||
|
||||
# Call the method
|
||||
response = await mlflow.save_model(input_data)
|
||||
|
||||
# Verify error handling
|
||||
assert response['success'] is False
|
||||
assert response['result'] is None
|
||||
assert 'Reports directory does not exist' in response['error_message']
|
||||
# Call the method - should raise exception
|
||||
with pytest.raises(FileNotFoundError, match='Reports directory does not exist'):
|
||||
await mlflow.save_model(input_data)
|
||||
|
||||
# Verify notification was sent
|
||||
mlflow.send_notification.assert_called_once_with(
|
||||
@@ -447,13 +439,9 @@ async def test_save_model_save_run_error(mlflow):
|
||||
'train_result': train_result,
|
||||
}
|
||||
|
||||
# Call the method
|
||||
response = await mlflow.save_model(input_data)
|
||||
|
||||
# Verify error handling
|
||||
assert response['success'] is False
|
||||
assert response['result'] is None
|
||||
assert 'One or more metrics (MSE, R2, MAE) are None' in response['error_message']
|
||||
# Call the method - should raise exception
|
||||
with pytest.raises(ValueError, match=r'One or more metrics \(MSE, R2, MAE\) are None'):
|
||||
await mlflow.save_model(input_data)
|
||||
|
||||
# Verify notification was sent
|
||||
mlflow.send_notification.assert_called_once_with(
|
||||
@@ -491,10 +479,9 @@ async def test_save_model_missing_metadata(mlflow):
|
||||
# Call the method
|
||||
response = await mlflow.save_model(input_data)
|
||||
|
||||
# Verify it still works (metadata defaults to {})
|
||||
assert response['success'] is True
|
||||
assert response['result'] == train_result
|
||||
assert response['error_message'] is None
|
||||
# Verify it still works (metadata defaults to {}) - returns TrainModelResult directly
|
||||
assert response == train_result
|
||||
assert response.run_name == 'test_experiment-1'
|
||||
|
||||
|
||||
@mark.asyncio
|
||||
@@ -540,10 +527,8 @@ async def test_save_model_complete_flow(mlflow):
|
||||
mlflow.model_monitoring_repository.generate_artifacts.assert_called_once()
|
||||
mlflow.model_monitoring_repository.save_run.assert_called_once_with(updated_result)
|
||||
|
||||
# Verify response
|
||||
assert response['success'] is True
|
||||
assert response['result'] == updated_result
|
||||
assert response['error_message'] is None
|
||||
assert updated_result.run_name == 'production_model-5'
|
||||
assert updated_result.run_dir is not None
|
||||
assert updated_result.report_path is not None
|
||||
# Verify response - returns TrainModelResult directly
|
||||
assert response == updated_result
|
||||
assert response.run_name == 'production_model-5'
|
||||
assert response.run_dir is not None
|
||||
assert response.report_path is not None
|
||||
|
||||
Reference in New Issue
Block a user