Added --allowed-hosts argument since it required in mlflow 3.10.1 - #11
Added --allowed-hosts argument since it required in mlflow 3.10.1#11MaheeGamage wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the MLflow container entrypoint to support MLflow 3.10.1 by adjusting how mlflow server CLI flags are assembled at runtime.
Changes:
- Refactors the entrypoint to build
mlflow serverarguments viaset --(instead of a fixedexec ... "$@"invocation). - Adds conditional support for passing
--allowed-hostsfromMLFLOW_ALLOWED_HOSTS.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| "$@" No newline at end of file | ||
| # Build the argument list dynamically so new optional flags can be added without branching. | ||
| # MLflow 3.x requires --allowed-hosts to be passed as a CLI flag (env var alone is insufficient). | ||
| set -- --host "$MLFLOW_HOST" --artifacts-destination "$MLFLOW_DEFAULT_ARTIFACTS_DESTINATION" |
There was a problem hiding this comment.
set -- overwrites the container's original positional arguments, so any args provided via Docker CMD / Kubernetes args (previously forwarded via "$@") are now dropped. This is a behavioral regression for anyone passing additional mlflow server flags; consider appending the original "$@" when building the new argument list so user-supplied args are preserved (and keep the intended override order explicit).
| set -- --host "$MLFLOW_HOST" --artifacts-destination "$MLFLOW_DEFAULT_ARTIFACTS_DESTINATION" | |
| set -- --host "$MLFLOW_HOST" --artifacts-destination "$MLFLOW_DEFAULT_ARTIFACTS_DESTINATION" "$@" |
| # Build the argument list dynamically so new optional flags can be added without branching. | ||
| # MLflow 3.x requires --allowed-hosts to be passed as a CLI flag (env var alone is insufficient). | ||
| set -- --host "$MLFLOW_HOST" --artifacts-destination "$MLFLOW_DEFAULT_ARTIFACTS_DESTINATION" | ||
| [ -n "$MLFLOW_ALLOWED_HOSTS" ] && set -- "$@" --allowed-hosts "$MLFLOW_ALLOWED_HOSTS" |
There was a problem hiding this comment.
The PR title says --allowed-hosts is required for mlflow 3.10.1, but this script only adds the flag when MLFLOW_ALLOWED_HOSTS is non-empty. There are no other repo references setting this env var, so the image will still start without the flag by default; if mlflow truly requires it, this change won't fix the startup failure. Consider either providing a safe default, or explicitly failing fast with a clear error when MLFLOW_ALLOWED_HOSTS is unset so deployments are forced to configure it.
No description provided.