Skip to content

Added --allowed-hosts argument since it required in mlflow 3.10.1 #11

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
MaheeGamage wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Added --allowed-hosts argument since it required in mlflow 3.10.1 #11

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
14 changes: 7 additions & 7 deletions mlflow/entrypoint.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@
# Exit immediately if a command exits with a non-zero status.
set -e

# Default values (can be overridden by environment variables if needed)
# Default values (can be overridden by environment variables)
MLFLOW_HOST="${MLFLOW_HOST:-0.0.0.0}"
MLFLOW_DEFAULT_ARTIFACTS_DESTINATION="${MLFLOW_DEFAULT_ARTIFACTS_DESTINATION:-s3://mlflow}"

# Start the MLflow server using exec to replace the shell process
# "$@" allows passing additional arguments from Kubernetes 'args' if needed in the future
exec mlflow server \
--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"
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

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).

Suggested change
set -- --host "$MLFLOW_HOST" --artifacts-destination "$MLFLOW_DEFAULT_ARTIFACTS_DESTINATION"
set -- --host "$MLFLOW_HOST" --artifacts-destination "$MLFLOW_DEFAULT_ARTIFACTS_DESTINATION" "$@"

Copilot uses AI. Check for mistakes.
[ -n "$MLFLOW_ALLOWED_HOSTS" ] && set -- "$@" --allowed-hosts "$MLFLOW_ALLOWED_HOSTS"
Comment on lines +10 to +13
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.

exec mlflow server "$@"
Loading