[mikewoodward94]

To fix this issue: #192

Mainly removing minio from the docker compose and adding LFS.

Readme is now out of date!

[github-actions]

Coverage Report

File	Stmts	Miss	Cover	Missing
mlops
Experiment.py	152	28	82%	21, 50–55, 78, 98, 100, 105, 159–170, 201, 203–204, 254–258, 262–263, 272–273
ProjectFile.py	23	1	96%	41
cli.py	55	17	69%	19–22, 36, 59–66, 86–88, 96–99, 107–108, 113
mlops/release
Release.py	33	7	79%	24, 28, 33–38
mlops/release/destinations
ReleaseDestination.py	9	1	89%	18
SharepointDestination.py	6	2	67%	12–13
ZenodoDestination.py	33	19	42%	21–25, 29–35, 43–60
mlops/release/sources
MLFlowSource.py	13	6	54%	13–14, 21–27
ReleaseSource.py	14	2	86%	14, 23
mlops/utils
Config.py	15	15	0%	1–22
TOTAL	390	98	75%

Tests	Skipped	Failures	Errors	Time
16	0 💤	0 ❌	0 🔥	11m 18s ⏱️

[mikaelsimard5]

I think this update should come with a massive disclaimer/release note because existing MLOps pipelines using a previous version of csc-mlops upgrading to this one will break if they have not ported their data accordingly. New data would work fine but old data would fail in the mlflow viewer as they're based on S3, so it should be mentioned in the release note that before upgrading to this, users should migrate their mlflow artifact store to a LFS and adjust postgres accordingly to allow visualisation of old experiments

[mikewoodward94]

I think this update should come with a massive disclaimer/release note because existing MLOps pipelines using a previous version of csc-mlops upgrading to this one will break if they have not ported their data accordingly. New data would work fine but old data would fail in the mlflow viewer as they're based on S3, so it should be mentioned in the release note that before upgrading to this, users should migrate their mlflow artifact store to a LFS and adjust postgres accordingly to allow visualisation of old experiments

We could add a failsafe check in Experiment.py that if it finds an s3 location set, it comes up with an error essentially saying this?

[mikaelsimard5]

Should also adjust the CLI default command accordingly as the default is still 8gb so anyone running mlops run() still gets the cli default. Adjust mlops/cli.py --shared_memory (line 45) to match?

[mikaelsimard5]

With the minio healthcheck removed it's not impossible that the docker-compose fails because it's may try to concurrently start mlflow and postgres processes which may be related. I think there needs to be a health check in the db service, so add a health check there such as

healthcheck:
test: ["CMD-SHELL", "psql -h 127.0.0.1 -U ${POSTGRES_USER} -d ${POSTGRES_DB} -Atc 'select 1'"]
interval: 5s
timeout: 5s
retries: 30

(basically makes sure the postgres db has been created before we move on to mlflow stuff).

[mikewoodward94]

Strong agree!