From f07bc8ff306f3c6ab8362111fe45a6b5e75a0613 Mon Sep 17 00:00:00 2001 From: Bruno Domingues Date: Wed, 22 Oct 2025 22:39:26 -0300 Subject: [PATCH] SIENTIAPDE-1241: Refactor: Remove redundant logging and fix duplicate logs This commit removes redundant logging statements from training and experiment tracking activities, preventing duplicate log entries. It also introduces a logger helper to disable log propagation, further addressing the duplicate logs issue. Additionally, the Makefile, run_coverage.sh, setup_port_forwards.sh, and simulator/Dockerfile files were removed as they are no longer needed. --- Makefile | 7 -- .../activities/experiment_tracking.py | 9 -- model_manager/activities/training.py | 7 -- model_manager/utils/logger_helper.py | 27 +++++ .../utils/repository/model_repository.py | 19 +-- .../utils/repository/storage_repository.py | 2 - .../utils/repository/training_repository.py | 17 +-- model_manager/worker/worker.py | 4 +- model_manager/workflows/train_model.py | 3 + run_coverage.sh | 11 -- setup_port_forwards.sh | 111 ------------------ simulator/Dockerfile | 30 ----- todo-list.txt | 1 - validate.sh | 47 +++++++- 14 files changed, 86 insertions(+), 209 deletions(-) delete mode 100644 Makefile create mode 100644 model_manager/utils/logger_helper.py delete mode 100755 run_coverage.sh delete mode 100755 setup_port_forwards.sh delete mode 100644 simulator/Dockerfile diff --git a/Makefile b/Makefile deleted file mode 100644 index f6e4208..0000000 --- a/Makefile +++ /dev/null @@ -1,7 +0,0 @@ -VERSION = 1.0.8 -name = sientia-model-manager -# ENVIRONMENT = production - -docker-hub: - @docker build --no-cache -t aignosi.azurecr.io/$(name):$(VERSION) . - @docker push aignosi.azurecr.io/$(name):$(VERSION) \ No newline at end of file diff --git a/model_manager/activities/experiment_tracking.py b/model_manager/activities/experiment_tracking.py index 027b69c..911de68 100644 --- a/model_manager/activities/experiment_tracking.py +++ b/model_manager/activities/experiment_tracking.py @@ -89,9 +89,6 @@ class ExperimentTracking(Postgres): notification_handler=notification_handler, ) - self.logger = logger - self.notification_handler = notification_handler - def __del__(self): """ Destructor to safely handle cleanup during garbage collection. @@ -160,10 +157,6 @@ class ExperimentTracking(Postgres): run_name = input_data.get('run_name') try: - self.info( - f'Updating experiment run {experiment_run_id} with status: {status}', metadata - ) - query_params: dict[str, Any] if update_type == UpdateType.STATUS: @@ -249,6 +242,4 @@ class ExperimentTracking(Postgres): level=NotificationLevel.ERROR, attachment_content=trace, ) - - self.error(trace, metadata=metadata) raise RuntimeError(error_msg) from e diff --git a/model_manager/activities/training.py b/model_manager/activities/training.py index 89bd537..d771d82 100644 --- a/model_manager/activities/training.py +++ b/model_manager/activities/training.py @@ -80,7 +80,6 @@ class Training(BaseActivity): metadata = input_data.get('metadata', {}) try: - self.info('Validating training parameters', metadata) train_params = TrainModelParams.from_dict(input_data) train_params.validate_business_rules() @@ -104,8 +103,6 @@ class Training(BaseActivity): level=NotificationLevel.ERROR, attachment_content=trace, ) - - self.error(trace, metadata=metadata) raise @activity.defn(name='train_model') @@ -177,8 +174,6 @@ class Training(BaseActivity): attachment_content=trace, ) - self.error(trace, metadata=metadata) - raise ModelTrainingError( model_trained=model_trained, model_saved=model_saved, @@ -223,6 +218,4 @@ class Training(BaseActivity): level=NotificationLevel.ERROR, attachment_content=trace, ) - - self.error(trace, metadata=metadata) raise diff --git a/model_manager/utils/logger_helper.py b/model_manager/utils/logger_helper.py new file mode 100644 index 0000000..b5fd47c --- /dev/null +++ b/model_manager/utils/logger_helper.py @@ -0,0 +1,27 @@ +""" +Logger helper to prevent duplicate logs caused by propagation. + +This module provides a wrapper around sientia_do Logger to disable +log propagation and prevent duplicate log entries in the Model Manager. +""" + +from sientia_do.observability.logger import Logger as SientiaLogger + + +def get_logger(name: str) -> SientiaLogger: + """ + Create a Logger instance with propagation disabled. + + This prevents duplicate logs caused by hierarchical propagation + in Python's logging system. + + Args: + name: Logger name (typically __name__ of the calling module). + + Returns: + Logger: Configured logger instance with propagation disabled. + """ + logger = SientiaLogger(name) + # Disable propagation to prevent duplicate logs + logger.base_logger.propagate = False + return logger diff --git a/model_manager/utils/repository/model_repository.py b/model_manager/utils/repository/model_repository.py index d9617f0..f878e36 100644 --- a/model_manager/utils/repository/model_repository.py +++ b/model_manager/utils/repository/model_repository.py @@ -55,25 +55,14 @@ class ModelRepository: Exception: If model saving fails (after sending notification) """ experiment_name = train_result.params.experiment_name - self.logger.info(f'Starting model save for experiment: {experiment_name}') - - # Step 1: Generate next run name - self.logger.info('Generating run name') train_result.run_name = self._get_next_run_name(experiment_name) - self.logger.info(f'Generated run name: {train_result.run_name}') - - # Step 2: Generate artifacts (reports, CSV files) - self.logger.info('Generating artifacts') train_result = self._generate_artifacts(train_result) - self.logger.info('Artifacts generated successfully') - - # Step 3: Save run to MLflow - self.logger.info('Saving run to MLflow') self._save_run(train_result) self.logger.info( - f'Model saved successfully - Run: {train_result.run_name}, ' - f'Experiment: {experiment_name}' + f'Model saved successfully - experiment run id: {train_result.params.experiment_run_id}, ' + f'experiment name: {experiment_name}, ' + f'run name: {train_result.run_name}' ) return train_result @@ -93,8 +82,6 @@ class ModelRepository: self.logger.info('No run directory specified, skipping cleanup') return - self.logger.info(f'Cleaning up run directory: {run_dir}') - if os.path.exists(run_dir): shutil.rmtree(run_dir) self.logger.info(f'Run directory deleted successfully: {run_dir}') diff --git a/model_manager/utils/repository/storage_repository.py b/model_manager/utils/repository/storage_repository.py index bb15017..71f3110 100644 --- a/model_manager/utils/repository/storage_repository.py +++ b/model_manager/utils/repository/storage_repository.py @@ -102,7 +102,6 @@ class StorageRepository: Raises: OSError: If the download fails (network, permissions, missing key, etc.). """ - self.logger.info(f'Fetching file from MinIO: {bucket_name}/{file_name}') response = self.minio_client.get_object(Bucket=bucket_name, Key=file_name) with response['Body'] as body: @@ -124,6 +123,5 @@ class StorageRepository: bucket_name: Bucket that contains the object. file_name: Object key to delete. """ - self.logger.info(f'Deleting file from MinIO: {bucket_name}/{file_name}') self.minio_client.delete_object(Bucket=bucket_name, Key=file_name) self.logger.info(f'File deleted successfully: {bucket_name}/{file_name}') diff --git a/model_manager/utils/repository/training_repository.py b/model_manager/utils/repository/training_repository.py index bf4115f..007e6eb 100644 --- a/model_manager/utils/repository/training_repository.py +++ b/model_manager/utils/repository/training_repository.py @@ -66,9 +66,7 @@ class TrainingRepository: ValueError: If transformed data is empty Exception: If data loading, preprocessing, or training fails """ - self.logger.info('Loading data from BytesIO file') data = load_data(uploaded_file, params.line_separator, params.decimal_separator) - self.logger.info('Initializing and fitting data preprocessor') process_data = self._init_data_preprocessor(params) process_data.fit(data) data_view = process_data.transform(data) @@ -76,7 +74,6 @@ class TrainingRepository: if len(data_view) <= 0: raise ValueError('Data view is empty after transformation') - self.logger.info('Splitting data into train/test sets') x_train, x_test, y_train, y_test = split_train_test( data_view[params.variable_columns], data_view[params.target_variable], @@ -85,17 +82,18 @@ class TrainingRepository: random_state=42, ) - self.logger.info('Preparing training data') data_train = pd.concat([x_train, y_train], axis=1) scaler_dict = self._init_scaler_dict(process_data, params) - self.logger.info('Training linear regression model') regr = LinearRegressionModel( target_variable=params.target_variable, variable_columns=params.variable_columns, ) regr.fit(data_train) + self.logger.info( + f'Model trained successfully - experiment run id: {params.experiment_run_id}' + ) return TrainModelResult( params=params, @@ -128,12 +126,10 @@ class TrainingRepository: TrainModelResult: Updated result with predictions, denormalized data, and metrics (mse_val, mae_val, r2_val) """ - self.logger.info('Making predictions on test set') y_pred_array = tmr.regr.predict(tmr.x_test) if params.use_scaler: scaler = tmr.process_data.get_scaler() - self.logger.info('Denormalizing features') # If using custom scaler with denormalize_* helpers if hasattr(scaler, 'denormalize_single_input'): @@ -141,7 +137,6 @@ class TrainingRepository: tmr.x_train[col] = scaler.denormalize_single_input(tmr.x_train[col], col) tmr.x_test[col] = scaler.denormalize_single_input(tmr.x_test[col], col) - self.logger.info('Denormalizing target variable') tmr.y_train = scaler.denormalize_single_input(tmr.y_train, params.target_variable) tmr.y_test = scaler.denormalize_single_input(tmr.y_test, params.target_variable) y_pred_array = scaler.denormalize_predictions(y_pred_array, params.target_variable) @@ -158,18 +153,15 @@ class TrainingRepository: tmr.x_test[feature_cols] = scaler.inverse_transform(x_test_features) # Target was not scaled with StandardScaler in preprocessing; leave y as-is - self.logger.info('Adding index to predictions') tmr.y_pred = pd.Series(y_pred_array, index=tmr.y_test.index) tmr.y_pred.name = f'{params.target_variable}_pred' - self.logger.info('Reordering all data by index') tmr.x_train = tmr.x_train.sort_index() tmr.x_test = tmr.x_test.sort_index() tmr.y_train = tmr.y_train.sort_index() tmr.y_test = tmr.y_test.sort_index() tmr.y_pred = tmr.y_pred.sort_index() - self.logger.info('Calculating evaluation metrics') assert tmr.y_pred is not None, 'y_pred should be set at this point' tmr.mse_val = round( @@ -183,6 +175,9 @@ class TrainingRepository: ) tmr.r2_val = round(r2(tmr.y_test.astype(np.float64), tmr.y_pred.astype(np.float64)), 2) + self.logger.info( + f'Model metrics calculated successfully - experiment run id: {params.experiment_run_id}' + ) return tmr def _init_scaler_dict(self, process_data: DataPreprocessor, params: TrainModelParams) -> dict: diff --git a/model_manager/worker/worker.py b/model_manager/worker/worker.py index ded2a79..ed553a4 100644 --- a/model_manager/worker/worker.py +++ b/model_manager/worker/worker.py @@ -33,7 +33,6 @@ with workflow.unsafe.imports_passed_through(): from prometheus_client import start_http_server from sientia_do.notifications.handlers import CoreNotificationHandler as NotificationHandler - from sientia_do.observability.logger import get_logger from model_manager import metrics from model_manager.activities.activities import Activities @@ -43,6 +42,7 @@ with workflow.unsafe.imports_passed_through(): build_mongodb_config, build_postgres_config, ) + from model_manager.utils.logger_helper import get_logger from model_manager.workflows.train_model import TrainModel POD_ID = os.getenv('POD_ID') @@ -73,7 +73,7 @@ async def main(): metadata = { 'pod_id': POD_ID, - 'workflow_name': '-', + 'workflow_name': 'train_model', } logger.custom_info(f'Starting Worker with POD_ID: {POD_ID}', metadata) diff --git a/model_manager/workflows/train_model.py b/model_manager/workflows/train_model.py index bdd5f22..6fa5eac 100644 --- a/model_manager/workflows/train_model.py +++ b/model_manager/workflows/train_model.py @@ -53,6 +53,8 @@ with workflow.unsafe.imports_passed_through(): maximum_attempts=5, ) +POD_ID = os.getenv('POD_ID') + @workflow.defn(name='train_model') class TrainModel: @@ -98,6 +100,7 @@ class TrainModel: metadata = { 'metadata': { + 'pod_id': POD_ID, 'experiment_run_id': experiment_run_id, 'workflow_name': 'train_model', } diff --git a/run_coverage.sh b/run_coverage.sh deleted file mode 100755 index 7d4f6f4..0000000 --- a/run_coverage.sh +++ /dev/null @@ -1,11 +0,0 @@ -#!/bin/bash - -# Exit on any error -set -e - -echo "Activating virtual environment..." -source ./venv/bin/activate - -pytest --cov=model_manager --cov-report=html - -xdg-open htmlcov/index.html \ No newline at end of file diff --git a/setup_port_forwards.sh b/setup_port_forwards.sh deleted file mode 100755 index 6717621..0000000 --- a/setup_port_forwards.sh +++ /dev/null @@ -1,111 +0,0 @@ -#!/bin/bash - -# Exit on any error -set -e - -# Color codes for output -RED='\033[0;31m' -GREEN='\033[0;32m' -YELLOW='\033[1;33m' -NC='\033[0m' # No Color - -echo -e "${GREEN}=== Port Forward Setup Script ===${NC}\n" - -# Define port forwards: LOCAL_PORT:NAMESPACE:SERVICE:REMOTE_PORT:DESCRIPTION -PORT_FORWARDS=( - "55432:paradedb:paradedb-rw:5432:PostgreSQL" - "45249:sientia-tracker:sientia-tracker-mlflow-tracking:80:MLflow" - "37463:temporal:temporal-frontend:7233:Temporal" - "8080:temporal:temporal-web:8080:Temporal UI" - "42297:mongodb:my-release-mongodb:27017:MongoDB" - "36577:minio:minio:9000:MinIO" -) - -# Step 1: Kill existing port-forward jobs for these services -echo -e "${YELLOW}Step 1: Checking for existing port-forward jobs...${NC}" - -for pf in "${PORT_FORWARDS[@]}"; do - IFS=':' read -r local_port namespace service remote_port description <<< "$pf" - - # Check if there's a job with this service name - existing_jobs=$(jobs -l | grep "kubectl.*port-forward.*svc/$service" || true) - - if [ -n "$existing_jobs" ]; then - echo -e "${YELLOW} Found existing port-forward for $description ($service)${NC}" - # Extract PIDs and kill them - pids=$(echo "$existing_jobs" | awk '{print $2}') - for pid in $pids; do - echo -e "${YELLOW} Killing job with PID $pid${NC}" - kill "$pid" 2>/dev/null || true - done - fi -done - -# Wait a moment for ports to be released -sleep 1 - -echo -e "${GREEN} Cleanup complete${NC}\n" - -# Step 2: Check if any of the ports are already in use -echo -e "${YELLOW}Step 2: Checking if ports are available...${NC}" - -ports_in_use=() - -for pf in "${PORT_FORWARDS[@]}"; do - IFS=':' read -r local_port namespace service remote_port description <<< "$pf" - - # Check if port is in use using lsof or netstat - if command -v lsof &> /dev/null; then - if lsof -Pi :$local_port -sTCP:LISTEN -t >/dev/null 2>&1; then - ports_in_use+=("$local_port:$description") - fi - elif command -v netstat &> /dev/null; then - if netstat -tuln | grep -q ":$local_port "; then - ports_in_use+=("$local_port:$description") - fi - elif command -v ss &> /dev/null; then - if ss -tuln | grep -q ":$local_port "; then - ports_in_use+=("$local_port:$description") - fi - fi -done - -# If any ports are in use, report and exit -if [ ${#ports_in_use[@]} -gt 0 ]; then - echo -e "${RED}ERROR: The following ports are already in use:${NC}" - for port_info in "${ports_in_use[@]}"; do - IFS=':' read -r port desc <<< "$port_info" - echo -e "${RED} - Port $port (for $desc)${NC}" - done - echo -e "\n${RED}Please free these ports before running this script.${NC}" - exit 1 -fi - -echo -e "${GREEN} All ports are available${NC}\n" - -# Step 3: Create all port forwards -echo -e "${YELLOW}Step 3: Creating port forwards...${NC}" - -for pf in "${PORT_FORWARDS[@]}"; do - IFS=':' read -r local_port namespace service remote_port description <<< "$pf" - - echo -e "${GREEN} Starting port-forward: $description${NC}" - echo -e " Local port: $local_port -> $namespace/$service:$remote_port" - - kubectl -n "$namespace" port-forward "svc/$service" "$local_port:$remote_port" & - - # Give it a moment to start - sleep 0.5 -done - -echo -e "\n${GREEN}=== All port forwards created successfully ===${NC}" -echo -e "\n${YELLOW}Active port forwards:${NC}" -for pf in "${PORT_FORWARDS[@]}"; do - IFS=':' read -r local_port namespace service remote_port description <<< "$pf" - echo -e " - ${GREEN}localhost:$local_port${NC} -> $description ($namespace/$service)" -done - -echo -e "\n${YELLOW}To stop all port forwards, run:${NC}" -echo -e " jobs -p | xargs kill" -echo -e "\n${YELLOW}To view active port forwards:${NC}" -echo -e " jobs -l" diff --git a/simulator/Dockerfile b/simulator/Dockerfile deleted file mode 100644 index d467676..0000000 --- a/simulator/Dockerfile +++ /dev/null @@ -1,30 +0,0 @@ -# syntax=docker/dockerfile:1.4 - -FROM python:3.11-slim - -# Enable use of SSH agent/socket -# This line enables SSH during build -# (don't forget the syntax header above) -RUN apt-get update && apt-get install -y git openssh-client && rm -rf /var/lib/apt/lists/* - -# Use build-time SSH mount for Git clone -# The SSH key will NOT remain in the image -# IMPORTANT: this block requires BuildKit -# and the --ssh flag during docker build - -# SSH config to skip host key check (safe in CI/local dev) -RUN mkdir -p /root/.ssh && echo "StrictHostKeyChecking no" > /root/.ssh/config - -WORKDIR /app - -# Clone using SSH -ARG GIT_REPO -ARG GIT_BRANCH=main - -# Mount SSH key just for this RUN -RUN --mount=type=ssh git clone --branch ${GIT_BRANCH} ${GIT_REPO} . - -# Install requirements if exists -RUN if [ -f requirements.txt ]; then pip install --no-cache-dir -r requirements.txt; fi - -CMD ["python", "server.py"] diff --git a/todo-list.txt b/todo-list.txt index 0b8cd92..e4a4fc2 100644 --- a/todo-list.txt +++ b/todo-list.txt @@ -1,3 +1,2 @@ - remover os testes das classes alteradas e refazer de novo depois - no final alterar o readme -- verificar erro de logs duplicados diff --git a/validate.sh b/validate.sh index e1d18b9..d4bbc15 100755 --- a/validate.sh +++ b/validate.sh @@ -1,9 +1,39 @@ #!/bin/bash # Model Manager Code Validation Script # This script runs all code quality checks before committing or deploying +# +# Usage: +# ./validate.sh # Run all checks including tests (default) +# ./validate.sh --no-tests # Skip unit tests +# ./validate.sh --skip-tests # Skip unit tests (alias) set -e # Exit on any error +# Parse command line arguments +RUN_TESTS=true +for arg in "$@"; do + case $arg in + --no-tests|--skip-tests) + RUN_TESTS=false + shift + ;; + --help|-h) + echo "Usage: $0 [OPTIONS]" + echo "" + echo "Options:" + echo " --no-tests, --skip-tests Skip unit tests (default: run tests)" + echo " --help, -h Show this help message" + echo "" + exit 0 + ;; + *) + echo "Unknown option: $arg" + echo "Use --help for usage information" + exit 1 + ;; + esac +done + # Colors for output RED='\033[0;31m' GREEN='\033[0;32m' @@ -16,6 +46,11 @@ echo -e "${BLUE}║ Model Manager - Code Validation Suite ║${N echo -e "${BLUE}╚════════════════════════════════════════════════════════╝${NC}" echo "" +if [ "$RUN_TESTS" = false ]; then + echo -e "${YELLOW}ℹ️ Unit tests will be skipped${NC}" + echo "" +fi + # Check if virtual environment is activated if [[ -z "${VIRTUAL_ENV}" ]] && [[ -z "${CONDA_DEFAULT_ENV}" ]]; then echo -e "${YELLOW}⚠️ Warning: No virtual environment detected${NC}" @@ -67,8 +102,16 @@ if ! run_step "4. Security Analysis (Bandit)" "bandit -r model_manager/ -ll -q"; fi # Step 5: Unit Tests (pytest) -if ! run_step "5. Unit Tests (pytest)" "pytest tests/ --cov=model_manager --cov-report=term-missing --cov-report=xml --cov-report=html --cov-fail-under=80 -q"; then - FAILED_STEPS+=("Unit Tests") +if [ "$RUN_TESTS" = true ]; then + if ! run_step "5. Unit Tests (pytest)" "pytest tests/ --cov=model_manager --cov-report=term-missing --cov-report=xml --cov-report=html --cov-fail-under=80 -q"; then + FAILED_STEPS+=("Unit Tests") + fi +else + echo -e "${BLUE}━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━${NC}" + echo -e "${BLUE}▶ 5. Unit Tests (pytest)${NC}" + echo -e "${BLUE}━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━${NC}" + echo -e "${YELLOW}⏭️ Unit Tests - SKIPPED${NC}" + echo "" fi # Summary