E2E testing for model promoting #36

Merged
vascodias23 merged 3 commits from staging-e2e-tests into main 2025-12-12 14:59:36 +00:00
vascodias23 commented 2025-12-11 15:20:27 +00:00 (Migrated from github.com)

End to end tests have been implemented, as well as a staging workflow. This allows for automated validation of the model before promoting it to production and deploying it.

End to end tests have been implemented, as well as a staging workflow. This allows for automated validation of the model before promoting it to production and deploying it.
AndreFerreira5 (Migrated from github.com) reviewed 2025-12-11 15:22:23 +00:00
@ -0,0 +1,94 @@
AndreFerreira5 (Migrated from github.com) commented 2025-12-11 15:22:23 +00:00

this step should be moved to another workflow file, like 4-continuous-production.yml

this step should be moved to another workflow file, like _4-continuous-production.yml_
copilot-pull-request-reviewer[bot] (Migrated from github.com) reviewed 2025-12-11 15:24:54 +00:00
copilot-pull-request-reviewer[bot] (Migrated from github.com) left a comment

Pull request overview

This pull request implements end-to-end testing and automated model promotion workflow for the ML pipeline. The changes enable automated validation of models in a staging environment before promoting them to production, enhancing the reliability of the deployment process. Key improvements include comprehensive E2E tests for the inference API, a model promotion script that leverages MLflow aliases, and a GitHub Actions workflow that orchestrates the staging validation and production deployment pipeline.

Key Changes

  • Added E2E test suite covering health checks, single/batch predictions, and error handling for the inference API
  • Created model promotion script to programmatically promote models from staging to production using MLflow
  • Implemented continuous staging workflow that validates models in staging, runs E2E tests, and automatically promotes successful models to production

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 9 comments.

File Description
tests/test_e2e/e2e_inference.py New E2E test suite for inference API with fixtures and test cases for health, predictions, and error handling
model-promotion/promote_model.py Script to promote MLflow models between aliases (staging to production) using environment configuration
.github/workflows/3-continuous-staging.yml Workflow orchestrating staging tests, model promotion, container tagging, and production deployment with health checks
Comments suppressed due to low confidence (2)

tests/test_e2e/e2e_inference.py:2

  • Import of 'requests' is not used.
import requests

tests/test_e2e/e2e_inference.py:4

  • Import of 'mlflow' is not used.
import mlflow
## Pull request overview This pull request implements end-to-end testing and automated model promotion workflow for the ML pipeline. The changes enable automated validation of models in a staging environment before promoting them to production, enhancing the reliability of the deployment process. Key improvements include comprehensive E2E tests for the inference API, a model promotion script that leverages MLflow aliases, and a GitHub Actions workflow that orchestrates the staging validation and production deployment pipeline. ### Key Changes - Added E2E test suite covering health checks, single/batch predictions, and error handling for the inference API - Created model promotion script to programmatically promote models from staging to production using MLflow - Implemented continuous staging workflow that validates models in staging, runs E2E tests, and automatically promotes successful models to production ### Reviewed changes Copilot reviewed 3 out of 3 changed files in this pull request and generated 9 comments. | File | Description | | ---- | ----------- | | tests/test_e2e/e2e_inference.py | New E2E test suite for inference API with fixtures and test cases for health, predictions, and error handling | | model-promotion/promote_model.py | Script to promote MLflow models between aliases (staging to production) using environment configuration | | .github/workflows/3-continuous-staging.yml | Workflow orchestrating staging tests, model promotion, container tagging, and production deployment with health checks | <details> <summary>Comments suppressed due to low confidence (2)</summary> **tests/test_e2e/e2e_inference.py:2** * Import of 'requests' is not used. ``` import requests ``` **tests/test_e2e/e2e_inference.py:4** * Import of 'mlflow' is not used. ``` import mlflow ``` </details>
@ -0,0 +1,94 @@
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2025-12-11 15:24:51 +00:00

Using a fixed 10-second sleep is a fragile approach for waiting for the service to be ready. The service might not be ready after 10 seconds in slow environments, or this could unnecessarily delay the workflow in fast environments. Consider implementing a retry loop that polls the health endpoint until it responds successfully or a timeout is reached, similar to the health check pattern used in lines 111-118.

          for i in {1..30}; do
            if curl -f http://localhost:9001/health | grep -q "healthy"; then
              echo "Staging service healthy"
              break
            fi
            sleep 2
          done
          # Fail if not healthy after retries
          if ! curl -f http://localhost:9001/health | grep -q "healthy"; then
            echo "Staging service failed to become healthy"
            exit 1
          fi
Using a fixed 10-second sleep is a fragile approach for waiting for the service to be ready. The service might not be ready after 10 seconds in slow environments, or this could unnecessarily delay the workflow in fast environments. Consider implementing a retry loop that polls the health endpoint until it responds successfully or a timeout is reached, similar to the health check pattern used in lines 111-118. ```suggestion for i in {1..30}; do if curl -f http://localhost:9001/health | grep -q "healthy"; then echo "Staging service healthy" break fi sleep 2 done # Fail if not healthy after retries if ! curl -f http://localhost:9001/health | grep -q "healthy"; then echo "Staging service failed to become healthy" exit 1 fi ```
@ -0,0 +1,24 @@
import os
import mlflow
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2025-12-11 15:24:54 +00:00

Import of 'mlflow' is not used.


Import of 'mlflow' is not used. ```suggestion ```
@ -0,0 +5,4 @@
def main():
client = MlflowClient(
tracking_uri=os.getenv("MLFLOW_TRACKING_URI"),
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2025-12-11 15:24:53 +00:00

The environment variable MLFLOW_TRACKING_URI is retrieved without validation. If this variable is not set, an MlflowClient will be created with None as the tracking URI, which may lead to unexpected behavior. Consider adding validation to ensure the variable is set and provide a clear error message if it's missing.

import sys
import mlflow
from mlflow.tracking import MlflowClient


def main():
    tracking_uri = os.getenv("MLFLOW_TRACKING_URI")
    if not tracking_uri:
        print("Error: The environment variable MLFLOW_TRACKING_URI is not set. Please set it to the URI of your MLflow tracking server.", file=sys.stderr)
        sys.exit(1)
    client = MlflowClient(
        tracking_uri=tracking_uri,
The environment variable `MLFLOW_TRACKING_URI` is retrieved without validation. If this variable is not set, an MlflowClient will be created with `None` as the tracking URI, which may lead to unexpected behavior. Consider adding validation to ensure the variable is set and provide a clear error message if it's missing. ```suggestion import sys import mlflow from mlflow.tracking import MlflowClient def main(): tracking_uri = os.getenv("MLFLOW_TRACKING_URI") if not tracking_uri: print("Error: The environment variable MLFLOW_TRACKING_URI is not set. Please set it to the URI of your MLflow tracking server.", file=sys.stderr) sys.exit(1) client = MlflowClient( tracking_uri=tracking_uri, ```
@ -0,0 +8,4 @@
tracking_uri=os.getenv("MLFLOW_TRACKING_URI"),
)
model_name = os.getenv("MLFLOW_MODEL_NAME")
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2025-12-11 15:24:53 +00:00

The environment variable MLFLOW_MODEL_NAME is retrieved without validation. If this variable is not set, the script will fail later with an unclear error message when calling get_model_version_by_alias. Consider adding validation to check if the variable exists and provide a clear error message if it's missing.

The environment variable `MLFLOW_MODEL_NAME` is retrieved without validation. If this variable is not set, the script will fail later with an unclear error message when calling `get_model_version_by_alias`. Consider adding validation to check if the variable exists and provide a clear error message if it's missing.
@ -0,0 +13,4 @@
to_alias = os.getenv("TO_ALIAS", "production")
# Get the version with the staging alias
model_version = client.get_model_version_by_alias(model_name, from_alias)
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2025-12-11 15:24:53 +00:00

If the model version with the specified alias doesn't exist, this call will raise an exception, but the error message may not be clear to users. Consider wrapping this in a try-except block to provide a more informative error message that explains which model and alias were being looked up.

If the model version with the specified alias doesn't exist, this call will raise an exception, but the error message may not be clear to users. Consider wrapping this in a try-except block to provide a more informative error message that explains which model and alias were being looked up.
@ -0,0 +1,86 @@
import pytest
import requests
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2025-12-11 15:24:52 +00:00

The requests module is imported but never used in this file. All HTTP requests are made through the FastAPI TestClient. Consider removing this unused import.


The `requests` module is imported but never used in this file. All HTTP requests are made through the FastAPI TestClient. Consider removing this unused import. ```suggestion ```
@ -0,0 +1,86 @@
import pytest
import requests
import pandas as pd
import mlflow
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2025-12-11 15:24:52 +00:00

The mlflow module is imported but never used in this file. Consider removing this unused import.


The `mlflow` module is imported but never used in this file. Consider removing this unused import. ```suggestion ```
@ -0,0 +63,4 @@
"data": [
{
"tpep_pickup_datetime": f"2013-01-01 {i:02d}:00:00",
"tpep_dropoff_datetime": f"2013-01-01 {i: 02d}:20:00",
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2025-12-11 15:24:52 +00:00

There is an inconsistent spacing in the f-string formatting. The format specifier has an extra space: {i: 02d} should be {i:02d} to match the formatting used on line 65.

                    "tpep_dropoff_datetime": f"2013-01-01 {i:02d}:20:00",
There is an inconsistent spacing in the f-string formatting. The format specifier has an extra space: `{i: 02d}` should be `{i:02d}` to match the formatting used on line 65. ```suggestion "tpep_dropoff_datetime": f"2013-01-01 {i:02d}:20:00", ```
@ -0,0 +83,4 @@
response = client.post("/predict", json=invalid_data)
# Should handle gracefully (400 or process with defaults)
assert response.status_code in [400, 200]
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2025-12-11 15:24:53 +00:00

The test for invalid input handling is too lenient by accepting both 400 and 200 status codes. This doesn't validate the actual error handling behavior. Consider making this test more specific: either assert that a 400 error is returned with a clear error message, or if 200 is valid, verify that the API handles missing fields with appropriate defaults and document this expected behavior.

The test for invalid input handling is too lenient by accepting both 400 and 200 status codes. This doesn't validate the actual error handling behavior. Consider making this test more specific: either assert that a 400 error is returned with a clear error message, or if 200 is valid, verify that the API handles missing fields with appropriate defaults and document this expected behavior.
AndreFerreira5 (Migrated from github.com) reviewed 2025-12-11 15:26:10 +00:00
@ -0,0 +1,94 @@
AndreFerreira5 (Migrated from github.com) commented 2025-12-11 15:26:10 +00:00

in a first version, we should just trigger model reloading instead of running a new container, because the first gh action file already builds and deploys the inference api whenever it detects a change

in a first version, we should just trigger model reloading instead of running a new container, because the first gh action file already builds and deploys the inference api whenever it detects a change
Sign in to join this conversation.
No description provided.