SIENTIAPDE-1243: Refactor and enhance model manager activities and workflows
This commit includes several changes: - Reorganized imports and class inheritance in activities.py, gates.py and mlflow.py for better readability and maintainability. - Improved error handling and logging in gates.py and mlflow.py. - Added input validation and filtering in gates.py to ensure data quality. - Enhanced prediction formatting and storage policy management in gates.py. - Updated metrics.py to use consistent naming conventions and labels. - Refactored connectors_config.py to use type hints and improve code clarity. - Updated conditional and MLFlow filters for better data quality checks. - Improved model repository logic for retraining and updating models. - Enhanced worker.py to include SDK metrics and improved error handling. - Refactored workflows for better modularity and error handling. - Updated tests to reflect the changes and improve test coverage.
This commit is contained in:
@@ -1,45 +1,42 @@
|
||||
from datetime import datetime
|
||||
from unittest.mock import ANY, MagicMock, patch
|
||||
|
||||
import numpy as np
|
||||
from pandas import DataFrame, Timestamp
|
||||
from pytest import fixture, mark, raises
|
||||
from sientia_do.temporal.constants import DATETIME_FORMAT, DATETIME_FORMAT_WITH_TZ
|
||||
from model_manager.activities.mlflow import MLFlow
|
||||
from pytest import fixture, mark
|
||||
from sientia_do.notifications.models import NotificationLevel
|
||||
from sientia_do.temporal.constants import DATETIME_FORMAT, DATETIME_FORMAT_WITH_TZ
|
||||
|
||||
from model_manager.activities.mlflow import MLFlow
|
||||
|
||||
|
||||
@patch("model_manager.activities.mlflow.MLFlowRepository")
|
||||
@patch('model_manager.activities.mlflow.MLFlowRepository')
|
||||
def test___init__(mock_mlflow_repository):
|
||||
mlflow = MLFlow(
|
||||
mlflow_host="http://localhost",
|
||||
mlflow_host='http://localhost',
|
||||
mlflow_port=5000,
|
||||
mlflow_username="admin",
|
||||
mlflow_password="admin",
|
||||
mlflow_username='admin',
|
||||
mlflow_password='admin',
|
||||
logger=MagicMock(),
|
||||
notification_handler=MagicMock()
|
||||
notification_handler=MagicMock(),
|
||||
)
|
||||
|
||||
assert mlflow.mlflow_host == "http://localhost"
|
||||
assert mlflow.mlflow_host == 'http://localhost'
|
||||
assert mlflow.mlflow_port == 5000
|
||||
assert mlflow.mlflow_username == "admin"
|
||||
assert mlflow.mlflow_password == "admin"
|
||||
assert mlflow.mlflow_username == 'admin'
|
||||
assert mlflow.mlflow_password == 'admin'
|
||||
|
||||
mock_mlflow_repository.assert_called_once_with(
|
||||
"http://localhost:5000", "admin", "admin", ANY
|
||||
)
|
||||
mock_mlflow_repository.assert_called_once_with('http://localhost:5000', 'admin', 'admin', ANY)
|
||||
|
||||
|
||||
@fixture
|
||||
@patch("model_manager.activities.mlflow.MLFlowRepository")
|
||||
@patch('model_manager.activities.mlflow.MLFlowRepository')
|
||||
def mlflow(mock_mlflow_repository):
|
||||
mlflow = MLFlow(
|
||||
mlflow_host="http://localhost:5000",
|
||||
mlflow_host='http://localhost:5000',
|
||||
mlflow_port=5000,
|
||||
mlflow_username="admin",
|
||||
mlflow_password="admin",
|
||||
mlflow_username='admin',
|
||||
mlflow_password='admin',
|
||||
logger=MagicMock(),
|
||||
notification_handler=MagicMock()
|
||||
notification_handler=MagicMock(),
|
||||
)
|
||||
|
||||
mlflow.send_notification = MagicMock()
|
||||
@@ -48,44 +45,67 @@ def mlflow(mock_mlflow_repository):
|
||||
|
||||
|
||||
metadata = {
|
||||
"metadata": {
|
||||
"model_id": "test_model",
|
||||
"model_name": "test_model",
|
||||
"workflow_name": "test_workflow",
|
||||
"schema_name": "test_schedule",
|
||||
'metadata': {
|
||||
'model_id': 'test_model',
|
||||
'model_name': 'test_model',
|
||||
'workflow_name': 'test_workflow',
|
||||
'schema_name': 'test_schedule',
|
||||
},
|
||||
}
|
||||
|
||||
|
||||
@mark.asyncio
|
||||
@patch("model_manager.activities.mlflow.DataFrame")
|
||||
@patch("model_manager.activities.mlflow.max")
|
||||
@patch('model_manager.activities.mlflow.DataFrame')
|
||||
@patch('model_manager.activities.mlflow.max')
|
||||
async def test_request_transform_success(mock_max, mock_dataframe, mlflow):
|
||||
mock_max.return_value = '2024-01-02'
|
||||
# Mock input data
|
||||
input_data = {
|
||||
**metadata,
|
||||
'data': [
|
||||
{'timestamp': '2024-01-01', 'variable': 'var1',
|
||||
'value': 1.0, 'created_at': '2024-01-01 12:00:00'},
|
||||
{'timestamp': '2024-01-01', 'variable': 'var2',
|
||||
'value': 2.0, 'created_at': '2024-01-01 12:00:00'},
|
||||
{'timestamp': '2024-01-02', 'variable': 'var1',
|
||||
'value': 3.0, 'created_at': '2024-01-02 12:00:00'},
|
||||
{'timestamp': '2024-01-02', 'variable': 'var2',
|
||||
'value': 4.0, 'created_at': '2024-01-02 12:00:00'},
|
||||
{'timestamp': '2024-01-02', 'variable': 'var1',
|
||||
'value': 1.0, 'created_at': '2024-01-01 12:00:00'},
|
||||
{'timestamp': '2024-01-02', 'variable': 'var2',
|
||||
'value': 1.0, 'created_at': '2024-01-01 12:00:00'}
|
||||
{
|
||||
'timestamp': '2024-01-01',
|
||||
'variable': 'var1',
|
||||
'value': 1.0,
|
||||
'created_at': '2024-01-01 12:00:00',
|
||||
},
|
||||
{
|
||||
'timestamp': '2024-01-01',
|
||||
'variable': 'var2',
|
||||
'value': 2.0,
|
||||
'created_at': '2024-01-01 12:00:00',
|
||||
},
|
||||
{
|
||||
'timestamp': '2024-01-02',
|
||||
'variable': 'var1',
|
||||
'value': 3.0,
|
||||
'created_at': '2024-01-02 12:00:00',
|
||||
},
|
||||
{
|
||||
'timestamp': '2024-01-02',
|
||||
'variable': 'var2',
|
||||
'value': 4.0,
|
||||
'created_at': '2024-01-02 12:00:00',
|
||||
},
|
||||
{
|
||||
'timestamp': '2024-01-02',
|
||||
'variable': 'var1',
|
||||
'value': 1.0,
|
||||
'created_at': '2024-01-01 12:00:00',
|
||||
},
|
||||
{
|
||||
'timestamp': '2024-01-02',
|
||||
'variable': 'var2',
|
||||
'value': 1.0,
|
||||
'created_at': '2024-01-01 12:00:00',
|
||||
},
|
||||
],
|
||||
'model_name': 'test_model',
|
||||
'model_config': {}
|
||||
'model_config': {},
|
||||
}
|
||||
|
||||
# Mock the transform response
|
||||
expected_response = {'prediction': [0.5, 0.6], 'timestamp': [
|
||||
'2024-01-01', '2024-01-02']}
|
||||
expected_response = {'prediction': [0.5, 0.6], 'timestamp': ['2024-01-01', '2024-01-02']}
|
||||
mlflow.model_monitoring_repository.transform.return_value = expected_response
|
||||
|
||||
mock_dataframe.return_value.sort_values.return_value = mock_dataframe.return_value
|
||||
@@ -114,30 +134,25 @@ async def test_request_transform_success(mock_max, mock_dataframe, mlflow):
|
||||
|
||||
|
||||
@mark.asyncio
|
||||
@patch("model_manager.activities.mlflow.DataFrame")
|
||||
@patch("model_manager.activities.mlflow.to_datetime")
|
||||
@patch("model_manager.activities.mlflow.max")
|
||||
@patch('model_manager.activities.mlflow.DataFrame')
|
||||
@patch('model_manager.activities.mlflow.to_datetime')
|
||||
@patch('model_manager.activities.mlflow.max')
|
||||
async def test_request_predict(mock_max, mock_to_datetime, mock_dataframe, mlflow):
|
||||
mock_max.return_value = '2024-01-02'
|
||||
# Mock input data
|
||||
input_data = {
|
||||
**metadata,
|
||||
'data': {
|
||||
"variable": {
|
||||
"2024-01-01": "var1",
|
||||
"2024-01-02": "var2",
|
||||
"2024-01-03": "var1",
|
||||
"2024-01-04": "var2"
|
||||
'variable': {
|
||||
'2024-01-01': 'var1',
|
||||
'2024-01-02': 'var2',
|
||||
'2024-01-03': 'var1',
|
||||
'2024-01-04': 'var2',
|
||||
},
|
||||
"value": {
|
||||
"2024-01-01": 1.0,
|
||||
"2024-01-02": 2.0,
|
||||
"2024-01-03": 3.0,
|
||||
"2024-01-04": 4.0
|
||||
}
|
||||
'value': {'2024-01-01': 1.0, '2024-01-02': 2.0, '2024-01-03': 3.0, '2024-01-04': 4.0},
|
||||
},
|
||||
'model_name': 'test_model',
|
||||
'model_config': {}
|
||||
'model_config': {},
|
||||
}
|
||||
|
||||
# Mock the predict response
|
||||
@@ -148,9 +163,7 @@ async def test_request_predict(mock_max, mock_to_datetime, mock_dataframe, mlflo
|
||||
response_data = await mlflow.request_predict(input_data)
|
||||
|
||||
mock_dataframe.assert_called_once_with(input_data['data'])
|
||||
mock_dataframe.return_value.replace.assert_called_once_with(
|
||||
np.nan, None, inplace=True
|
||||
)
|
||||
mock_dataframe.return_value.replace.assert_called_once_with(np.nan, None, inplace=True)
|
||||
mock_dataframe.return_value.__setitem__.assert_any_call(
|
||||
'timestamp', mock_to_datetime.return_value.dt.strftime.return_value
|
||||
)
|
||||
@@ -158,9 +171,7 @@ async def test_request_predict(mock_max, mock_to_datetime, mock_dataframe, mlflo
|
||||
mock_to_datetime.assert_called_once_with(
|
||||
mock_dataframe.return_value.__getitem__.return_value, format=DATETIME_FORMAT_WITH_TZ
|
||||
)
|
||||
mock_to_datetime.return_value.dt.strftime.assert_called_once_with(
|
||||
DATETIME_FORMAT
|
||||
)
|
||||
mock_to_datetime.return_value.dt.strftime.assert_called_once_with(DATETIME_FORMAT)
|
||||
|
||||
# Verify the response
|
||||
assert response_data == expected_response
|
||||
@@ -174,28 +185,26 @@ async def test_request_predict(mock_max, mock_to_datetime, mock_dataframe, mlflo
|
||||
@mark.asyncio
|
||||
async def test_retrain_model(mlflow):
|
||||
data = {
|
||||
"model_id": [4, 5, 6, 7],
|
||||
"created_at": [1, 2, 3, 4],
|
||||
"timestamp": [1, 1, 2, 2],
|
||||
"variable": ["var1", "var2", "var1", "var2"],
|
||||
"value": [1, 2, 3, 4]
|
||||
'model_id': [4, 5, 6, 7],
|
||||
'created_at': [1, 2, 3, 4],
|
||||
'timestamp': [1, 1, 2, 2],
|
||||
'variable': ['var1', 'var2', 'var1', 'var2'],
|
||||
'value': [1, 2, 3, 4],
|
||||
}
|
||||
|
||||
mlflow.model_monitoring_repository.retrain_model.return_value = (
|
||||
'Model retrained successfully', 'test')
|
||||
'Model retrained successfully',
|
||||
'test',
|
||||
)
|
||||
|
||||
response = await mlflow.retrain_model({
|
||||
**metadata,
|
||||
'data': data,
|
||||
'model_name': 'test_model'
|
||||
})
|
||||
response = await mlflow.retrain_model({**metadata, 'data': data, 'model_name': 'test_model'})
|
||||
|
||||
mlflow.model_monitoring_repository.retrain_model.assert_called_once()
|
||||
|
||||
assert response == {
|
||||
"status": 'Model retrained successfully',
|
||||
"timestamp": 2,
|
||||
"experiment": 'test'
|
||||
'status': 'Model retrained successfully',
|
||||
'timestamp': 2,
|
||||
'experiment': 'test',
|
||||
}
|
||||
|
||||
|
||||
@@ -206,20 +215,16 @@ async def test_retrain_model_error(mlflow):
|
||||
)
|
||||
|
||||
data = {
|
||||
"model_id": [4, 5, 6, 7],
|
||||
"created_at": [1, 2, 3, 4],
|
||||
"timestamp": [1, 1, 2, 2],
|
||||
"variable": ["var1", "var2", "var1", "var2"],
|
||||
"value": [1, 2, 3, 4]
|
||||
'model_id': [4, 5, 6, 7],
|
||||
'created_at': [1, 2, 3, 4],
|
||||
'timestamp': [1, 1, 2, 2],
|
||||
'variable': ['var1', 'var2', 'var1', 'var2'],
|
||||
'value': [1, 2, 3, 4],
|
||||
}
|
||||
|
||||
try:
|
||||
await mlflow.retrain_model({
|
||||
**metadata,
|
||||
'data': data,
|
||||
'model_name': 'test_model'
|
||||
})
|
||||
except Exception as e:
|
||||
await mlflow.retrain_model({**metadata, 'data': data, 'model_name': 'test_model'})
|
||||
except Exception as e: # noqa: BLE001
|
||||
assert str(e) == 'Error retraining model'
|
||||
mlflow.send_notification.assert_called_once_with(
|
||||
metadata=metadata['metadata'],
|
||||
@@ -227,20 +232,18 @@ async def test_retrain_model_error(mlflow):
|
||||
message='Error retraining model test_model: Error retraining model',
|
||||
block='retrain_model',
|
||||
level=NotificationLevel.ERROR,
|
||||
attachment_content=ANY
|
||||
attachment_content=ANY,
|
||||
)
|
||||
else:
|
||||
assert False, "No exception raised"
|
||||
raise AssertionError('No exception raised')
|
||||
|
||||
|
||||
@mark.asyncio
|
||||
async def test_update_production_model(mlflow):
|
||||
mlflow.model_monitoring_repository.update_production_model.return_value = (
|
||||
{
|
||||
"data1": 1,
|
||||
"data2": 2
|
||||
}
|
||||
)
|
||||
mlflow.model_monitoring_repository.update_production_model.return_value = {
|
||||
'data1': 1,
|
||||
'data2': 2,
|
||||
}
|
||||
|
||||
input_data = {
|
||||
**metadata,
|
||||
@@ -248,13 +251,14 @@ async def test_update_production_model(mlflow):
|
||||
'model_id': 1,
|
||||
'experiment': 'test',
|
||||
'timestamp': 2,
|
||||
'status': 'success'
|
||||
'status': 'success',
|
||||
}
|
||||
|
||||
response = await mlflow.update_production_model(input_data)
|
||||
|
||||
mlflow.model_monitoring_repository.update_production_model.assert_called_once_with(
|
||||
experiment='test', model_name='test_model')
|
||||
experiment='test', model_name='test_model'
|
||||
)
|
||||
|
||||
assert response == {
|
||||
'data1': {0: 1},
|
||||
@@ -262,7 +266,7 @@ async def test_update_production_model(mlflow):
|
||||
'model_id': {0: 1},
|
||||
'model_name': {0: 'test_model'},
|
||||
'timestamp': {0: 2},
|
||||
'status': {0: 'success'}
|
||||
'status': {0: 'success'},
|
||||
}
|
||||
|
||||
|
||||
@@ -278,12 +282,12 @@ async def test_update_production_model_error(mlflow):
|
||||
'model_id': 1,
|
||||
'experiment': 'test',
|
||||
'timestamp': 2,
|
||||
'status': 'success'
|
||||
'status': 'success',
|
||||
}
|
||||
|
||||
try:
|
||||
await mlflow.update_production_model(input_data)
|
||||
except Exception as e:
|
||||
except Exception as e: # noqa: BLE001
|
||||
assert str(e) == 'Error updating production model'
|
||||
mlflow.send_notification.assert_called_once_with(
|
||||
metadata=metadata['metadata'],
|
||||
@@ -291,7 +295,7 @@ async def test_update_production_model_error(mlflow):
|
||||
message='Error updating production model test_model: Error updating production model',
|
||||
block='update_production_model',
|
||||
level=NotificationLevel.ERROR,
|
||||
attachment_content=ANY
|
||||
attachment_content=ANY,
|
||||
)
|
||||
else:
|
||||
assert False, "No exception raised"
|
||||
raise AssertionError('No exception raised')
|
||||
|
||||
Reference in New Issue
Block a user