E2E testing for model promoting #36
No reviewers
Labels
No labels
CI/CD
bug
documentation
duplicate
enhancement
good first issue
help wanted
invalid
question
tests
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
AndreFerreira5/downtown-cab-co!36
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "staging-e2e-tests"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
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.
@ -0,0 +1,94 @@this step should be moved to another workflow file, like 4-continuous-production.yml
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
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 9 comments.
Comments suppressed due to low confidence (2)
tests/test_e2e/e2e_inference.py:2
tests/test_e2e/e2e_inference.py:4
@ -0,0 +1,94 @@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.
@ -0,0 +1,24 @@import osimport mlflowImport of 'mlflow' is not used.
@ -0,0 +5,4 @@def main():client = MlflowClient(tracking_uri=os.getenv("MLFLOW_TRACKING_URI"),The environment variable
MLFLOW_TRACKING_URIis retrieved without validation. If this variable is not set, an MlflowClient will be created withNoneas 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.@ -0,0 +8,4 @@tracking_uri=os.getenv("MLFLOW_TRACKING_URI"),)model_name = os.getenv("MLFLOW_MODEL_NAME")The environment variable
MLFLOW_MODEL_NAMEis retrieved without validation. If this variable is not set, the script will fail later with an unclear error message when callingget_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 aliasmodel_version = client.get_model_version_by_alias(model_name, from_alias)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 pytestimport requestsThe
requestsmodule is imported but never used in this file. All HTTP requests are made through the FastAPI TestClient. Consider removing this unused import.@ -0,0 +1,86 @@import pytestimport requestsimport pandas as pdimport mlflowThe
mlflowmodule is imported but never used in this file. Consider removing this unused import.@ -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",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.@ -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]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.
@ -0,0 +1,94 @@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