Skip to content
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
1dd07f5
Add --include_path (or -i) option to copy a folder into the build con…
mikaelsimard5 Mar 25, 2026
8aeeaea
Fix test_cli to use installed python version (3.9) rather than system…
mikaelsimard5 Mar 25, 2026
7563aa7
upgrade actions/setup-python as v2 may not be robust
mikaelsimard5 Mar 25, 2026
e9d4797
Add debug step to check Python version
mikaelsimard5 Mar 25, 2026
08ee6bd
Second debug step to verify python version before env clean
mikaelsimard5 Mar 25, 2026
9d2fa59
Moved clear runner space before installing python; otherwise it wipes…
mikaelsimard5 Mar 25, 2026
a621e79
fix mlflow==2.10.0 in test requirements to match csc-mlops requirements
mikaelsimard5 Mar 25, 2026
4549483
debugging step to understand why minio not working
mikaelsimard5 Mar 25, 2026
88eb617
adjusted deprecated command from createbuckets service and locked the…
mikaelsimard5 Mar 25, 2026
8954a94
Removed debugging step from test_cli
mikaelsimard5 Mar 25, 2026
faef3e7
Re-add createbuckets debug logging
mikaelsimard5 Mar 25, 2026
507f9da
Replace debug step with explicit wait for MLOps server readiness
mikaelsimard5 Mar 25, 2026
9ad30fe
Adjusting wait period
mikaelsimard5 Mar 25, 2026
d036d4b
ask minio if healthy before pursuing
mikaelsimard5 Mar 25, 2026
5aef93e
Wait for createbuckets to finish before running CLI test
mikaelsimard5 Mar 25, 2026
9860407
Wait for createbuckets using docker compose logs (blocks until contai…
mikaelsimard5 Mar 25, 2026
5931775
New theory - the issue is that createbucket was not waiting for minio…
mikaelsimard5 Mar 25, 2026
aca5575
robust settings for web
mikaelsimard5 Mar 25, 2026
65e7f97
Revert to last known working state
mikaelsimard5 Mar 25, 2026
832a76f
Restore robust MLOps server startup with proper depends_on conditions…
mikaelsimard5 Mar 25, 2026
a76a39e
removed redundant conditions in docker-compose
mikaelsimard5 Mar 26, 2026
e9ce063
removed --wait flag to understand why the test is now failing
mikaelsimard5 Mar 26, 2026
ca6e7f5
Clear runner space and add --wait to pull_request_tests
mikaelsimard5 Mar 26, 2026
c42301c
Moved include-path to experiment init; added robustness checks
mikaelsimard5 Mar 31, 2026
48bd4db
Renamed method to add paths so it doesn't conflict with the actual va…
mikaelsimard5 Mar 31, 2026
bccf904
Adjusted logic; if dest folder exists, then just point it out to user…
mikaelsimard5 Mar 31, 2026
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 10 additions & 7 deletions .github/workflows/test_cli.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,26 +33,29 @@ jobs:
fetch-depth: 0
ref: ${{ github.event.pull_request.head.ref }}

- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v2
with:
python-version: ${{ matrix.python-version }}

- name: Clear runner space
run: |
rm -rf /opt/hostedtoolcache

- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v5
with:
python-version: ${{ matrix.python-version }}

- name: Install csc-mlops
run: |
python -m pip install --upgrade pip build
python -m build
pip install dist/*.whl
python -m pip install dist/*.whl

- name: start MLOps server
# --wait blocks until all services are healthy or completed before returning.
# createbuckets depends on minio being healthy, so the mlflow bucket is guaranteed
# to exist before the test runs.
run: |
mkdir -p ~/MLOps_data/mlflow_minio
mkdir -p ~/MLOps_data/mlflow_db
docker compose -f mlflow_server/docker-compose.yml up -d --build
docker compose -f mlflow_server/docker-compose.yml up -d --build --wait

- name: Test run CLI
run: |
Expand Down
15 changes: 10 additions & 5 deletions mlflow_server/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,10 @@ services:
target: /data

createbuckets:
image: minio/mc
image: minio/mc:RELEASE.2022-11-07T23-47-39Z
depends_on:
- minio
minio:
condition: service_healthy
environment:
- NO_PROXY=minio,db,localhost
networks:
Expand All @@ -54,7 +55,7 @@ services:
/bin/sh -c "
/usr/bin/mc alias set myminio http://minio:9000 ${AWS_ACCESS_KEY_ID} ${AWS_SECRET_ACCESS_KEY};
/usr/bin/mc mb myminio/mlflow;
/usr/bin/mc policy set public myminio/mlflow;
/usr/bin/mc anonymous set public myminio/mlflow;
exit 0;"

db:
Expand All @@ -81,8 +82,12 @@ services:
image: mlflow_server
container_name: mlflow_server
depends_on:
- "minio"
- "db"
minio:
condition: service_healthy
db:
condition: service_started
createbuckets:
condition: service_completed_successfully
networks:
- frontend
- backend
Expand Down
11 changes: 11 additions & 0 deletions mlops/Experiment.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import atexit
import configparser
import logging
import os
import shutil
import subprocess
import sys

Expand Down Expand Up @@ -76,7 +78,7 @@
if self.auth is None:
logger.debug(f'Found minio credentials in {self.auth.method}')
raise Exception(
f'minio credentials not found - either specify in ~/.aws/credentials or using environment variables (AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY)')

Check failure on line 81 in mlops/Experiment.py

View workflow job for this annotation

GitHub Actions / unit_tests (ubuntu-latest, 3.9)

E501 line too long (160 > 127 characters)

def check_dirty(self) -> bool:
"""
Expand Down Expand Up @@ -247,6 +249,15 @@
"""
rebuild_docker = kwargs.get('rebuild_docker', False)
shared_memory = kwargs.get('shared_memory', '8gb')
include_path = kwargs.get('include_path', None)

if include_path:
folder_name = os.path.basename(os.path.abspath(include_path))
included_folder_dest = os.path.join(self.project_path, folder_name)
shutil.rmtree(included_folder_dest, ignore_errors=True)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this could result in accidental folder deletions, say someone accidentally set the include_path as "config" it would then delete the config folder (and do so silently).

Could we have a check_include_path() function that's called at init if the variable is passed? Think it just needs to do the following two things, and then can raise easy to action useful errors!

  1. Makes sure the path exists to include:
if not os.path.exists(include_path):
    raise FileNotFoundError(f"Source folder does not exist: {include_path}")
  1. Makes sure the destination folder doesn't already exist:
if os.path.exists(included_folder_dest):
    raise FileExistsError(f"Destination folder already exists: {included_folder_dest}. Please remove it first or rename your source folder.")

Feel free to add to this for anything else you think of thats worth checking.

Copy link
Copy Markdown
Contributor Author

@mikaelsimard5 mikaelsimard5 Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

I think in general it makes more sense to perform this action while initiating the experiment (similarly to the check_dirty, env_setup, ... methods), so I moved the action from run() to init, and added the suggested checks - see c42301c (main changes), and 48bd4db and bccf904 (two small bug fixes to the logic implemented in c42301c).

shutil.copytree(include_path, included_folder_dest)
logger.info(f'Copied {include_path} to {included_folder_dest}')
atexit.register(shutil.rmtree, included_folder_dest, ignore_errors=True) # remove folder at exit

logger.info(f'Starting experiment: {self.experiment_name}')

Expand Down
4 changes: 3 additions & 1 deletion mlops/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ def cli(ctx):
show_default=True, default=False)
@click.option('--ignore_git_check', is_flag=True, show_default=True, default=False,
help='TESTING ONLY - ignore git checks, occasionally it might be necessary to ignore the git checks for example, offline testing, do not use this feature if working on tracked models')
def run(script, config_path, run_name, ignore_git_check, shared_memory, logging_level, rebuild_docker):
@click.option('-i', '--include_path', 'include_path', help='Path to a folder to copy into the project directory before building the Docker image', default=None, type=click.Path(exists=True, file_okay=False))
def run(script, config_path, run_name, ignore_git_check, shared_memory, logging_level, rebuild_docker, include_path):
"""
Runs python project using csc-mlops framework.

Expand All @@ -65,6 +66,7 @@ def run(script, config_path, run_name, ignore_git_check, shared_memory, logging_
run_name=run_name,
rebuild_docker=rebuild_docker,
shared_memory=shared_memory,
include_path=include_path,
)


Expand Down
2 changes: 1 addition & 1 deletion tests/data/requirements.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
numpy
-f https://download.pytorch.org/whl/torch_stable.html
torch==2.0.1+cpu
mlflow
mlflow==2.10.0
boto3
requests==2.31.0
Loading