SIENTIAPDE-1255: Implement object destructors for Activities and ExperimentTracking to prevent AttributeError during garbage collection, and mock workflow logger in tests to avoid RuntimeWarning.
This commit is contained in:
@@ -102,6 +102,26 @@ class Activities(ExperimentTracking, MLFlow, MinIO, Training):
|
||||
|
||||
Training.__init__(self, logger=logger, notification_handler=notification_handler)
|
||||
|
||||
def __del__(self):
|
||||
"""
|
||||
Destructor to safely handle cleanup during garbage collection.
|
||||
|
||||
This prevents AttributeError when the parent Postgres.__del__ tries to access
|
||||
self.engine in objects with multiple inheritance. Only attempts cleanup if
|
||||
the engine attribute exists.
|
||||
"""
|
||||
# Only call parent __del__ if engine attribute exists
|
||||
# This prevents AttributeError in multiple inheritance scenarios
|
||||
if hasattr(self, 'engine'):
|
||||
try:
|
||||
# Call parent class __del__ if it exists
|
||||
if hasattr(super(), '__del__'):
|
||||
super().__del__()
|
||||
except Exception: # noqa: S110, BLE001
|
||||
# Silently ignore errors during garbage collection
|
||||
# Logging here could cause issues if logger is already destroyed
|
||||
pass
|
||||
|
||||
async def shutdown(self):
|
||||
"""
|
||||
Gracefully shutdown all activities and clean up resources.
|
||||
@@ -112,5 +132,7 @@ class Activities(ExperimentTracking, MLFlow, MinIO, Training):
|
||||
|
||||
The method should be called before the application terminates to ensure
|
||||
proper resource cleanup and prevent resource leaks.
|
||||
|
||||
Prefer calling this method explicitly rather than relying on __del__.
|
||||
"""
|
||||
ExperimentTracking.close(self)
|
||||
|
||||
@@ -88,6 +88,23 @@ class ExperimentTracking(Postgres):
|
||||
self.logger = logger
|
||||
self.notification_handler = notification_handler
|
||||
|
||||
def __del__(self):
|
||||
"""
|
||||
Destructor to safely handle cleanup during garbage collection.
|
||||
|
||||
This prevents AttributeError when used in multiple inheritance scenarios
|
||||
where the parent Postgres.__del__ might be called on objects without
|
||||
the engine attribute.
|
||||
"""
|
||||
# Only call parent __del__ if engine attribute exists
|
||||
if hasattr(self, 'engine'):
|
||||
try:
|
||||
if hasattr(super(), '__del__'):
|
||||
super().__del__()
|
||||
except Exception: # noqa: S110, BLE001
|
||||
# Silently ignore errors during garbage collection
|
||||
pass
|
||||
|
||||
@activity.defn(name='update_experiment_run')
|
||||
async def update_experiment_run(self, input_data: dict[str, Any]) -> None:
|
||||
"""
|
||||
|
||||
@@ -89,6 +89,9 @@ async def test_run_success_complete_flow(
|
||||
'removed_intervals': [],
|
||||
}
|
||||
|
||||
# Mock workflow.logger to avoid RuntimeWarning about unawaited coroutines
|
||||
workflow_mock.logger = MagicMock()
|
||||
|
||||
# Mock activity responses
|
||||
workflow_mock.execute_activity_method = AsyncMock(
|
||||
side_effect=[
|
||||
@@ -264,6 +267,9 @@ async def test_validate_training_parameters_validation_error(
|
||||
input_data = {'experiment_run_id': 123}
|
||||
metadata = {'metadata': {'experiment_run_id': 123, 'workflow_name': 'train_model'}}
|
||||
|
||||
# Mock workflow.logger to avoid RuntimeWarning about unawaited coroutines
|
||||
workflow_mock.logger = MagicMock()
|
||||
|
||||
workflow_mock.execute_activity_method = AsyncMock(
|
||||
side_effect=[
|
||||
ValueError('Missing required field'), # validate_train_params fails
|
||||
@@ -318,6 +324,9 @@ async def test_download_and_train_model_download_error(
|
||||
"""Test download error is handled correctly."""
|
||||
metadata = {'metadata': {'experiment_run_id': 123, 'workflow_name': 'train_model'}}
|
||||
|
||||
# Mock workflow.logger to avoid RuntimeWarning about unawaited coroutines
|
||||
workflow_mock.logger = MagicMock()
|
||||
|
||||
workflow_mock.execute_activity_method = AsyncMock(
|
||||
side_effect=[
|
||||
Exception('MinIO connection failed'), # fetch_file_from_minio fails
|
||||
@@ -342,6 +351,9 @@ async def test_download_and_train_model_training_error(
|
||||
"""Test training error is handled correctly."""
|
||||
metadata = {'metadata': {'experiment_run_id': 123, 'workflow_name': 'train_model'}}
|
||||
|
||||
# Mock workflow.logger to avoid RuntimeWarning about unawaited coroutines
|
||||
workflow_mock.logger = MagicMock()
|
||||
|
||||
workflow_mock.execute_activity_method = AsyncMock(
|
||||
side_effect=[
|
||||
b'file_content', # fetch_file_from_minio succeeds
|
||||
@@ -396,6 +408,7 @@ async def test_download_and_train_model_closes_bytesio_on_error(
|
||||
"""Test that BytesIO is closed in finally block even on error."""
|
||||
metadata = {'metadata': {'experiment_run_id': 123, 'workflow_name': 'train_model'}}
|
||||
|
||||
workflow_mock.logger = MagicMock()
|
||||
# Create a mock BytesIO with close method
|
||||
mock_file = MagicMock()
|
||||
mock_file.close = MagicMock()
|
||||
@@ -425,6 +438,7 @@ async def test_download_and_train_model_handles_file_without_close(
|
||||
"""Test that workflow handles file objects without close method gracefully."""
|
||||
metadata = {'metadata': {'experiment_run_id': 123, 'workflow_name': 'train_model'}}
|
||||
|
||||
workflow_mock.logger = MagicMock()
|
||||
# Create a mock file without close method
|
||||
mock_file = MagicMock(spec=[])
|
||||
|
||||
@@ -457,6 +471,7 @@ async def test_save_model_to_mlflow_success(
|
||||
"""Test successful model saving to MLFlow."""
|
||||
metadata = {'metadata': {'experiment_run_id': 123, 'workflow_name': 'train_model'}}
|
||||
|
||||
workflow_mock.logger = MagicMock()
|
||||
workflow_mock.execute_activity_method = AsyncMock(
|
||||
side_effect=[
|
||||
mock_train_result, # save_model
|
||||
@@ -480,6 +495,7 @@ async def test_save_model_to_mlflow_error(
|
||||
"""Test MLFlow save error is handled correctly."""
|
||||
metadata = {'metadata': {'experiment_run_id': 123, 'workflow_name': 'train_model'}}
|
||||
|
||||
workflow_mock.logger = MagicMock()
|
||||
workflow_mock.execute_activity_method = AsyncMock(
|
||||
side_effect=[
|
||||
Exception('MLFlow connection failed'), # save_model fails
|
||||
@@ -511,6 +527,7 @@ async def test_cleanup_resources_success(
|
||||
"""Test successful resource cleanup."""
|
||||
metadata = {'metadata': {'experiment_run_id': 123, 'workflow_name': 'train_model'}}
|
||||
|
||||
workflow_mock.logger = MagicMock()
|
||||
workflow_mock.execute_activity_method = AsyncMock(
|
||||
side_effect=[
|
||||
None, # cleanup_run_directory
|
||||
@@ -537,6 +554,7 @@ async def test_cleanup_resources_delete_error(
|
||||
"""Test cleanup handles delete errors correctly."""
|
||||
metadata = {'metadata': {'experiment_run_id': 123, 'workflow_name': 'train_model'}}
|
||||
|
||||
workflow_mock.logger = MagicMock()
|
||||
workflow_mock.execute_activity_method = AsyncMock(
|
||||
side_effect=[
|
||||
None, # cleanup_run_directory succeeds
|
||||
|
||||
Reference in New Issue
Block a user