Mlflow Release Pipeline #44
Conversation
diamonwiggins
commented
Apr 8, 2025
•
diamonwiggins
commented
- Adds release pipeline using GH Actions
- Adds Taskfile for dev/test/release tasks
- Adds tests for Mlflow
- Adds docs for the Helm Chart
fix app slug fix helm setup action fix helm version add ct.yaml bump upload-artifact version fix kubeconfig retrieval bump chart testing version fix typo ci improvements and adding kots install remove conditional from create release pass license file as content pass license as content modify app slug for unstable channel fix slug move some steps to make targets add replicated helm install to pipeline fix chart testing setup fix makefile debug ci debug ci fix ci fix ci use jq to parse customer json debug ci try getting oci working with chart testing fix ct install kubeconfig offset channel name by run number in case of re-runs add more distros revert adding distros cleanup add test for mlflow fix mlflow test domain fix chart file path fix helm install stop using chart-testing fix helm install enable application tests debug remove make make sure helm dependencies get updated fix helm install bump timeout on test script and increase cluster size use kubectl port forward to access app fix port forward wait before port-forward fix authentication in test script fix test add application testing for kots install improve wait for mlfow service before starting test moving to taskfile moving to taskfile commit taskfile fix taskfile remove check deps fix get licenseid fix get licenseid fix get licenseid fix get licenseid fix get licenseid fix port forward fix port forward add missing dependency for tests refactor taskfile polish readme
diamonwiggins
force-pushed
the
diamonwiggins/release-pipeline
branch
from
8304186 to
fcd8d87
Compare
| task add:repos:helm | ||
| task update:deps:helm |
There was a problem hiding this comment.
Interesting is this the actual command? So rather than helm-add-repos or helm-update-deps the command had ":" in it? Is that because of how the tasks are organized/imported or just an intentional naming decision?
I don't know that I have any particular opinion on the right way but this at least looked different enough to me to stand out.
There was a problem hiding this comment.
The LLM chose this naming scheme, and since this is my first time using Taskfile, i thought this was idiomatic to Taskfile in some way 😆
| - task: forward:port | ||
|
|
||
| # Port forwarding task | ||
| forward:port: |
There was a problem hiding this comment.
This is a really big task.
I wonder if you would be better served to split of the status checking into a utility task. Something that you could re-use in other tasks as well if you're just 'waiting for deploy' or something.
| - echo "Running application tests against MLflow on localhost:{{.PORT}}..." | ||
| - | | ||
| echo "Installing Python dependencies for tests..." | ||
| pip3 install setuptools mlflow==2.11.0 pandas>=2.0.0 scikit-learn>=1.3.0 requests>=2.31.0 urllib3>=2.0.0 |
There was a problem hiding this comment.
Ouch this doesn't look like how I would want to manage my test dependencies :)
pyproject.toml or at least a requirements file that get's installed would be a lot easier to maintain and update. There is tooling that can process those where as this is going to always require manual updates.
There was a problem hiding this comment.
yeah, for sure. wouldn't leave it this way, but have plans to give some more thought to the tests part of this in a follow up PR.
There was a problem hiding this comment.
I'm fine with test tasks being follow on from here, maybe leave a note in a comment to make that clear to future travelers.
| @@ -0,0 +1,267 @@ | |||
| #!/usr/bin/env python3 | |||
There was a problem hiding this comment.
If you're going to use python for testing, wouldn't using pytest clean things up some? It would handle printing test results, provide a modular system to define tests, and you can just lean into the other python tooling for dependencies and environments.
There was a problem hiding this comment.
yeah, agreed. there's a mlflow_example.py in the repo and I basically had it(the robots) convert that into a "test", but wasn't too prescriptive on that part yet. This is in my notes to create a follow up story to make this better.
There was a problem hiding this comment.
As above, I'm fine with testing being a follow up task with a note somewhere that it's still under development.
| sh: helm show chart ./charts/infra | grep '^version:' | cut -d ' ' -f 2 | ||
|
|
||
| # Release configuration | ||
| APP_NAME: diamon-mlflow |
There was a problem hiding this comment.
It is hardcode. Should we change to APP_NAME: '{{.APP_NAME | default "diamon-mlflow"}}'?
or APP_NAME: '{{.APP_NAME | default ""}}' and add check afterward
There was a problem hiding this comment.
I'll also point out that Ash made a change that I merged into wg_easy branch to move from APP_NAME to whatever it is the replicated cli is using. REPLICATED_APP or something like that? Check the latest wg-easy branch.
