Merge branch 'main' into release/SIENTIAPDE-1645
This commit is contained in:
@@ -4,7 +4,7 @@ import asyncio
|
||||
import os
|
||||
import shutil
|
||||
import tempfile
|
||||
from datetime import UTC, datetime, timedelta
|
||||
from datetime import datetime, timedelta
|
||||
from importlib import reload
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
@@ -34,15 +34,6 @@ def mock_metrics_controller():
|
||||
return controller
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def mock_storage_repository():
|
||||
"""Fixture for a mock storage repository."""
|
||||
repo = MagicMock()
|
||||
repo.delete_file = MagicMock()
|
||||
repo.list_bucket_objects = MagicMock()
|
||||
return repo
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def temp_dir():
|
||||
"""Fixture to create and clean up a temporary directory."""
|
||||
@@ -59,11 +50,9 @@ def temp_dir():
|
||||
{
|
||||
'CLEANUP_RETENTION_HOURS': '24',
|
||||
'CLEANUP_DRY_RUN': 'false',
|
||||
'MAX_KEYS_CLEANUP': '1000',
|
||||
},
|
||||
)
|
||||
def test_cleanup_init_default_values(
|
||||
mock_storage_repository,
|
||||
mock_logger,
|
||||
mock_notification_handler,
|
||||
mock_metrics_controller,
|
||||
@@ -75,7 +64,6 @@ def test_cleanup_init_default_values(
|
||||
from model_manager.activities.cleanup import Cleanup
|
||||
|
||||
cleanup = Cleanup(
|
||||
storage_repository=mock_storage_repository,
|
||||
logger=mock_logger,
|
||||
notification_handler=mock_notification_handler,
|
||||
metrics_controller=mock_metrics_controller,
|
||||
@@ -83,11 +71,9 @@ def test_cleanup_init_default_values(
|
||||
|
||||
assert cleanup.retention_hours == 24
|
||||
assert cleanup.dry_run is False
|
||||
assert cleanup.max_keys_cleanup == 1000
|
||||
|
||||
|
||||
def test_cleanup_init_custom_env_values(
|
||||
mock_storage_repository,
|
||||
mock_logger,
|
||||
mock_notification_handler,
|
||||
mock_metrics_controller,
|
||||
@@ -98,7 +84,6 @@ def test_cleanup_init_custom_env_values(
|
||||
{
|
||||
'CLEANUP_RETENTION_HOURS': '48',
|
||||
'CLEANUP_DRY_RUN': 'true',
|
||||
'MAX_KEYS_CLEANUP': '500',
|
||||
},
|
||||
):
|
||||
import model_manager.activities.cleanup
|
||||
@@ -106,16 +91,14 @@ def test_cleanup_init_custom_env_values(
|
||||
reload(model_manager.activities.cleanup)
|
||||
from model_manager.activities.cleanup import Cleanup
|
||||
|
||||
cleanup = Cleanup(
|
||||
storage_repository=mock_storage_repository,
|
||||
logger=mock_logger,
|
||||
notification_handler=mock_notification_handler,
|
||||
metrics_controller=mock_metrics_controller,
|
||||
)
|
||||
cleanup = Cleanup(
|
||||
logger=mock_logger,
|
||||
notification_handler=mock_notification_handler,
|
||||
metrics_controller=mock_metrics_controller,
|
||||
)
|
||||
|
||||
assert cleanup.retention_hours == 48
|
||||
assert cleanup.dry_run is True
|
||||
assert cleanup.max_keys_cleanup == 500
|
||||
assert cleanup.retention_hours == 48
|
||||
assert cleanup.dry_run is True
|
||||
|
||||
|
||||
@patch.dict(os.environ, {'CLEANUP_RETENTION_HOURS': 'invalid'})
|
||||
@@ -127,162 +110,10 @@ def test_cleanup_init_invalid_env_value_raises_error():
|
||||
reload(model_manager.activities.cleanup)
|
||||
|
||||
|
||||
# --- MinIO Cleanup Tests ---
|
||||
|
||||
|
||||
def test_cleanup_minio_files_missing_bucket_name(
|
||||
mock_storage_repository,
|
||||
mock_logger,
|
||||
mock_notification_handler,
|
||||
mock_metrics_controller,
|
||||
):
|
||||
"""Test cleanup_minio_files raises ValueError if bucket_name is missing."""
|
||||
from model_manager.activities.cleanup import Cleanup
|
||||
|
||||
cleanup = Cleanup(
|
||||
storage_repository=mock_storage_repository,
|
||||
logger=mock_logger,
|
||||
notification_handler=mock_notification_handler,
|
||||
metrics_controller=mock_metrics_controller,
|
||||
)
|
||||
cleanup._emit_metrics = AsyncMock()
|
||||
|
||||
with pytest.raises(ValueError, match='bucket_name must be provided'):
|
||||
asyncio.run(cleanup.cleanup_minio_files({'metadata': {}}))
|
||||
|
||||
|
||||
@patch.dict('model_manager.activities.cleanup.os.environ', {'CLEANUP_DRY_RUN': 'false'})
|
||||
def test_cleanup_minio_files_success_with_deletions(
|
||||
mock_storage_repository,
|
||||
mock_logger,
|
||||
mock_notification_handler,
|
||||
mock_metrics_controller,
|
||||
):
|
||||
"""Test successful deletion of old files from MinIO."""
|
||||
import model_manager.activities.cleanup
|
||||
|
||||
reload(model_manager.activities.cleanup)
|
||||
from model_manager.activities.cleanup import Cleanup
|
||||
|
||||
cleanup = Cleanup(
|
||||
storage_repository=mock_storage_repository,
|
||||
logger=mock_logger,
|
||||
notification_handler=mock_notification_handler,
|
||||
metrics_controller=mock_metrics_controller,
|
||||
)
|
||||
cleanup._emit_metrics = AsyncMock()
|
||||
|
||||
old_ts = int((datetime.now(UTC) - timedelta(hours=48)).timestamp() * 1000)
|
||||
recent_ts = int((datetime.now(UTC) - timedelta(hours=1)).timestamp() * 1000)
|
||||
|
||||
mock_storage_repository.list_bucket_objects.return_value = [
|
||||
f'{old_ts}-old-file.txt',
|
||||
f'{recent_ts}-recent-file.txt',
|
||||
'no-timestamp-file.txt',
|
||||
]
|
||||
|
||||
asyncio.run(cleanup.cleanup_minio_files({'bucket_name': 'test-bucket', 'metadata': {}}))
|
||||
|
||||
mock_storage_repository.delete_file.assert_called_once_with(
|
||||
'test-bucket', f'{old_ts}-old-file.txt'
|
||||
)
|
||||
cleanup._emit_metrics.assert_called_once()
|
||||
|
||||
|
||||
def test_cleanup_minio_files_dry_run(
|
||||
mock_storage_repository,
|
||||
mock_logger,
|
||||
mock_notification_handler,
|
||||
mock_metrics_controller,
|
||||
):
|
||||
"""Test MinIO cleanup in dry_run mode does not delete files."""
|
||||
with patch.dict(os.environ, {'CLEANUP_DRY_RUN': 'true'}):
|
||||
import model_manager.activities.cleanup
|
||||
|
||||
reload(model_manager.activities.cleanup)
|
||||
from model_manager.activities.cleanup import Cleanup
|
||||
|
||||
cleanup = Cleanup(
|
||||
storage_repository=mock_storage_repository,
|
||||
logger=mock_logger,
|
||||
notification_handler=mock_notification_handler,
|
||||
metrics_controller=mock_metrics_controller,
|
||||
)
|
||||
cleanup._emit_metrics = AsyncMock()
|
||||
|
||||
old_ts = int((datetime.now(UTC) - timedelta(hours=48)).timestamp() * 1000)
|
||||
mock_storage_repository.list_bucket_objects.return_value = [f'{old_ts}-old-file.txt']
|
||||
|
||||
asyncio.run(cleanup.cleanup_minio_files({'bucket_name': 'test-bucket', 'metadata': {}}))
|
||||
|
||||
mock_storage_repository.delete_file.assert_not_called()
|
||||
cleanup._emit_metrics.assert_called_once()
|
||||
|
||||
|
||||
@patch.dict('model_manager.activities.cleanup.os.environ', {'CLEANUP_DRY_RUN': 'false'})
|
||||
def test_cleanup_minio_files_delete_error(
|
||||
mock_storage_repository,
|
||||
mock_logger,
|
||||
mock_notification_handler,
|
||||
mock_metrics_controller,
|
||||
):
|
||||
"""Test error during MinIO file deletion is handled gracefully."""
|
||||
import model_manager.activities.cleanup
|
||||
|
||||
reload(model_manager.activities.cleanup)
|
||||
from model_manager.activities.cleanup import Cleanup
|
||||
|
||||
cleanup = Cleanup(
|
||||
storage_repository=mock_storage_repository,
|
||||
logger=mock_logger,
|
||||
notification_handler=mock_notification_handler,
|
||||
metrics_controller=mock_metrics_controller,
|
||||
)
|
||||
cleanup._emit_metrics = AsyncMock()
|
||||
cleanup.error = MagicMock()
|
||||
|
||||
old_ts = int((datetime.now(UTC) - timedelta(hours=48)).timestamp() * 1000)
|
||||
mock_storage_repository.list_bucket_objects.return_value = [f'{old_ts}-old-file.txt']
|
||||
mock_storage_repository.delete_file.side_effect = OSError('Permission Denied')
|
||||
|
||||
asyncio.run(cleanup.cleanup_minio_files({'bucket_name': 'test-bucket', 'metadata': {}}))
|
||||
|
||||
cleanup.error.assert_called_once()
|
||||
cleanup._emit_metrics.assert_called_once()
|
||||
|
||||
|
||||
def test_cleanup_minio_files_exception_handling(
|
||||
mock_storage_repository,
|
||||
mock_logger,
|
||||
mock_notification_handler,
|
||||
mock_metrics_controller,
|
||||
):
|
||||
"""Test exception during MinIO cleanup triggers notification and metrics."""
|
||||
from model_manager.activities.cleanup import Cleanup
|
||||
|
||||
cleanup = Cleanup(
|
||||
storage_repository=mock_storage_repository,
|
||||
logger=mock_logger,
|
||||
notification_handler=mock_notification_handler,
|
||||
metrics_controller=mock_metrics_controller,
|
||||
)
|
||||
cleanup.send_notification = MagicMock()
|
||||
cleanup._emit_metrics = AsyncMock()
|
||||
|
||||
mock_storage_repository.list_bucket_objects.side_effect = Exception('Connection Error')
|
||||
|
||||
with pytest.raises(Exception, match='Connection Error'):
|
||||
asyncio.run(cleanup.cleanup_minio_files({'bucket_name': 'test-bucket', 'metadata': {}}))
|
||||
|
||||
cleanup.send_notification.assert_called_once()
|
||||
cleanup._emit_metrics.assert_called_once()
|
||||
|
||||
|
||||
# --- Temp Directory Cleanup Tests ---
|
||||
|
||||
|
||||
def test_cleanup_temp_directories_nonexistent_path(
|
||||
mock_storage_repository,
|
||||
mock_logger,
|
||||
mock_notification_handler,
|
||||
mock_metrics_controller,
|
||||
@@ -291,7 +122,6 @@ def test_cleanup_temp_directories_nonexistent_path(
|
||||
from model_manager.activities.cleanup import Cleanup
|
||||
|
||||
cleanup = Cleanup(
|
||||
storage_repository=mock_storage_repository,
|
||||
logger=mock_logger,
|
||||
notification_handler=mock_notification_handler,
|
||||
metrics_controller=mock_metrics_controller,
|
||||
@@ -310,7 +140,6 @@ def test_cleanup_temp_directories_nonexistent_path(
|
||||
@patch.dict('model_manager.activities.cleanup.os.environ', {'CLEANUP_DRY_RUN': 'false'})
|
||||
def test_cleanup_temp_directories_success_with_deletions(
|
||||
temp_dir,
|
||||
mock_storage_repository,
|
||||
mock_logger,
|
||||
mock_notification_handler,
|
||||
mock_metrics_controller,
|
||||
@@ -322,7 +151,6 @@ def test_cleanup_temp_directories_success_with_deletions(
|
||||
from model_manager.activities.cleanup import Cleanup
|
||||
|
||||
cleanup = Cleanup(
|
||||
storage_repository=mock_storage_repository,
|
||||
logger=mock_logger,
|
||||
notification_handler=mock_notification_handler,
|
||||
metrics_controller=mock_metrics_controller,
|
||||
@@ -347,7 +175,6 @@ def test_cleanup_temp_directories_success_with_deletions(
|
||||
@patch.dict(os.environ, {'CLEANUP_DRY_RUN': 'true'})
|
||||
def test_cleanup_temp_directories_dry_run(
|
||||
temp_dir,
|
||||
mock_storage_repository,
|
||||
mock_logger,
|
||||
mock_notification_handler,
|
||||
mock_metrics_controller,
|
||||
@@ -359,7 +186,6 @@ def test_cleanup_temp_directories_dry_run(
|
||||
from model_manager.activities.cleanup import Cleanup
|
||||
|
||||
cleanup = Cleanup(
|
||||
storage_repository=mock_storage_repository,
|
||||
logger=mock_logger,
|
||||
notification_handler=mock_notification_handler,
|
||||
metrics_controller=mock_metrics_controller,
|
||||
@@ -379,7 +205,6 @@ def test_cleanup_temp_directories_dry_run(
|
||||
@patch.dict('model_manager.activities.cleanup.os.environ', {'CLEANUP_DRY_RUN': 'false'})
|
||||
def test_cleanup_temp_directories_delete_error(
|
||||
temp_dir,
|
||||
mock_storage_repository,
|
||||
mock_logger,
|
||||
mock_notification_handler,
|
||||
mock_metrics_controller,
|
||||
@@ -391,7 +216,6 @@ def test_cleanup_temp_directories_delete_error(
|
||||
from model_manager.activities.cleanup import Cleanup
|
||||
|
||||
cleanup = Cleanup(
|
||||
storage_repository=mock_storage_repository,
|
||||
logger=mock_logger,
|
||||
notification_handler=mock_notification_handler,
|
||||
metrics_controller=mock_metrics_controller,
|
||||
@@ -414,7 +238,6 @@ def test_cleanup_temp_directories_delete_error(
|
||||
|
||||
|
||||
def test_emit_metrics(
|
||||
mock_storage_repository,
|
||||
mock_logger,
|
||||
mock_notification_handler,
|
||||
mock_metrics_controller,
|
||||
@@ -423,7 +246,6 @@ def test_emit_metrics(
|
||||
from model_manager.activities.cleanup import Cleanup
|
||||
|
||||
cleanup = Cleanup(
|
||||
storage_repository=mock_storage_repository,
|
||||
logger=mock_logger,
|
||||
notification_handler=mock_notification_handler,
|
||||
metrics_controller=mock_metrics_controller,
|
||||
@@ -444,7 +266,6 @@ def test_emit_metrics(
|
||||
|
||||
def test_cleanup_temp_directories_with_files_and_unmatched_dirs(
|
||||
temp_dir,
|
||||
mock_storage_repository,
|
||||
mock_logger,
|
||||
mock_notification_handler,
|
||||
mock_metrics_controller,
|
||||
@@ -456,7 +277,6 @@ def test_cleanup_temp_directories_with_files_and_unmatched_dirs(
|
||||
from model_manager.activities.cleanup import Cleanup
|
||||
|
||||
cleanup = Cleanup(
|
||||
storage_repository=mock_storage_repository,
|
||||
logger=mock_logger,
|
||||
notification_handler=mock_notification_handler,
|
||||
metrics_controller=mock_metrics_controller,
|
||||
@@ -480,7 +300,6 @@ def test_cleanup_temp_directories_with_files_and_unmatched_dirs(
|
||||
|
||||
def test_cleanup_temp_directories_invalid_timestamp_format(
|
||||
temp_dir,
|
||||
mock_storage_repository,
|
||||
mock_logger,
|
||||
mock_notification_handler,
|
||||
mock_metrics_controller,
|
||||
@@ -492,7 +311,6 @@ def test_cleanup_temp_directories_invalid_timestamp_format(
|
||||
from model_manager.activities.cleanup import Cleanup
|
||||
|
||||
cleanup = Cleanup(
|
||||
storage_repository=mock_storage_repository,
|
||||
logger=mock_logger,
|
||||
notification_handler=mock_notification_handler,
|
||||
metrics_controller=mock_metrics_controller,
|
||||
@@ -512,7 +330,6 @@ def test_cleanup_temp_directories_invalid_timestamp_format(
|
||||
|
||||
def test_cleanup_temp_directories_generic_exception(
|
||||
temp_dir,
|
||||
mock_storage_repository,
|
||||
mock_logger,
|
||||
mock_notification_handler,
|
||||
mock_metrics_controller,
|
||||
@@ -524,7 +341,6 @@ def test_cleanup_temp_directories_generic_exception(
|
||||
from model_manager.activities.cleanup import Cleanup
|
||||
|
||||
cleanup = Cleanup(
|
||||
storage_repository=mock_storage_repository,
|
||||
logger=mock_logger,
|
||||
notification_handler=mock_notification_handler,
|
||||
metrics_controller=mock_metrics_controller,
|
||||
@@ -541,7 +357,6 @@ def test_cleanup_temp_directories_generic_exception(
|
||||
|
||||
|
||||
def test_emit_metrics_activity_only(
|
||||
mock_storage_repository,
|
||||
mock_logger,
|
||||
mock_notification_handler,
|
||||
mock_metrics_controller,
|
||||
@@ -550,7 +365,6 @@ def test_emit_metrics_activity_only(
|
||||
from model_manager.activities.cleanup import Cleanup
|
||||
|
||||
cleanup = Cleanup(
|
||||
storage_repository=mock_storage_repository,
|
||||
logger=mock_logger,
|
||||
notification_handler=mock_notification_handler,
|
||||
metrics_controller=mock_metrics_controller,
|
||||
|
||||
@@ -342,7 +342,6 @@ def test_train_model_training_fails(
|
||||
):
|
||||
"""Test train_model when training fails."""
|
||||
from model_manager.activities.training import Training
|
||||
from model_manager.utils.exceptions import ModelTrainingError
|
||||
|
||||
training = Training(
|
||||
model_repository=mock_model_repository,
|
||||
@@ -362,11 +361,10 @@ def test_train_model_training_fails(
|
||||
'train_params': mock_train_params,
|
||||
}
|
||||
|
||||
with pytest.raises(ModelTrainingError) as exc_info:
|
||||
with pytest.raises(RuntimeError) as exc_info:
|
||||
asyncio.run(training.train_model(input_data))
|
||||
|
||||
assert exc_info.value.model_trained is False
|
||||
assert exc_info.value.model_saved is False
|
||||
assert str(exc_info.value) == 'Training failed'
|
||||
training.send_notification.assert_called_once()
|
||||
|
||||
|
||||
@@ -382,7 +380,6 @@ def test_train_model_save_fails(
|
||||
):
|
||||
"""Test train_model when model saving fails."""
|
||||
from model_manager.activities.training import Training
|
||||
from model_manager.utils.exceptions import ModelTrainingError
|
||||
|
||||
training = Training(
|
||||
model_repository=mock_model_repository,
|
||||
@@ -406,11 +403,10 @@ def test_train_model_save_fails(
|
||||
'train_params': mock_train_params,
|
||||
}
|
||||
|
||||
with pytest.raises(ModelTrainingError) as exc_info:
|
||||
with pytest.raises(RuntimeError) as exc_info:
|
||||
asyncio.run(training.train_model(input_data))
|
||||
|
||||
assert exc_info.value.model_trained is True
|
||||
assert exc_info.value.model_saved is False
|
||||
assert str(exc_info.value) == 'Save failed'
|
||||
training.send_notification.assert_called_once()
|
||||
|
||||
|
||||
@@ -437,14 +433,11 @@ def test_cleanup_resources_success(
|
||||
input_data = {
|
||||
'metadata': {'workflow_id': 'test-123'},
|
||||
'run_dir': '/tmp/run_001', # noqa: S108
|
||||
'bucket_name': 'test-bucket',
|
||||
'file_name': 'test-file.csv',
|
||||
}
|
||||
|
||||
asyncio.run(training.cleanup_resources(input_data))
|
||||
|
||||
mock_model_repository.cleanup_run_directory.assert_called_once_with('/tmp/run_001') # noqa: S108
|
||||
mock_storage_repository.delete_file.assert_called_once_with('test-bucket', 'test-file.csv')
|
||||
|
||||
|
||||
@patch('model_manager.activities.training.TrainingRepository')
|
||||
@@ -510,4 +503,3 @@ def test_cleanup_resources_with_empty_values(
|
||||
asyncio.run(training.cleanup_resources(input_data))
|
||||
|
||||
mock_model_repository.cleanup_run_directory.assert_called_once_with('')
|
||||
mock_storage_repository.delete_file.assert_called_once_with('', '')
|
||||
|
||||
Reference in New Issue
Block a user