Repository navigation
Fix environment variable expansion in docker/README.md - #25474
Conversation
Signed-off-by: CoralGarden52 <[email protected]>
github-actions
Bot
added
area/docs
Install mlflow from this PR
Install mlflow from this PR# mlflow
pip install git+https://github.com/mlflow/mlflow.git@refs/pull/25474/merge
# mlflow-skinny
pip install git+https://github.com/mlflow/mlflow.git@refs/pull/25474/merge#subdirectory=libs/skinnyFor Databricks, use the following command: %sh curl -LsSf https://raw.githubusercontent.com/mlflow/mlflow/HEAD/dev/install-skinny.sh | sh -s pull/25474/mergePR author's recent activityIn the last 14 days, @CoralGarden52 opened 12 PRs across 8 repos:
|
harupy
left a comment
harupy
left a comment
There was a problem hiding this comment.
The fix is correct, and worth stating explicitly since it is not obvious: docker run execs the command argv without a shell, so $MLFLOW_BACKEND_STORE_URI was never going to expand there. There is no shell in that context to expand it, and the container's own env is not consulted for argv. Relying on mlflow server reading the env var directly is the right minimal fix.
One correction to the description, since the squash-merged commit message inherits it. The body says an unset variable "leaves MLflow with an empty --backend-store-uri value". Click actually consumes the next token as the option value, so the old form aborted with:
$ mlflow server --backend-store-uri --host 0.0.0.0
Error: Got unexpected extra argument (0.0.0.0)
with backend_store_uri set to the string --host. Worth fixing because Got unexpected extra argument is the message users will actually search for, and because the empty-value framing hides that the server would also have silently fallen back to binding 127.0.0.1 had there been no trailing argument.
Signed-off-by: CoralGarden52 <[email protected]>
Related Issues/PRs
N/A
What changes are proposed in this pull request?
The Docker README passes
MLFLOW_BACKEND_STORE_URIinto the container, but the MySQL, PostgreSQL, and Docker Compose examples also reference$MLFLOW_BACKEND_STORE_URIin the command. The directdocker runcommand is expanded by the host shell, while Docker executes the resulting command arguments without a shell inside the container. Docker Compose also interpolates variables before the container starts, so the container environment is not used for this expansion. WhenMLFLOW_BACKEND_STORE_URIis unset in the host environment, the command becomesmlflow server --backend-store-uri --host 0.0.0.0; Click consumes--hostas the value of--backend-store-uriand then fails withGot unexpected extra argument (0.0.0.0). Removing the redundant option lets MLflow readMLFLOW_BACKEND_STORE_URIfrom the container environment directly.How is this PR tested?
Existing unit/integration tests
New unit/integration tests
Manual tests
Reproduced the host-shell argument expansion with an unset
MLFLOW_BACKEND_STORE_URIand verified the corrected command arguments.Confirmed all affected README examples no longer use the host-expanded variable.
Ran
npx --yes [email protected] --check docker/README.md.Ran
git diff --check.Does this PR require documentation update?
Does this PR require updating the MLflow Skills repository?
Release Notes
Is this a user-facing change?
MLFLOW_BACKEND_STORE_URIenvironment variable.What component(s), interfaces, languages, and integrations does this PR affect?
Components
area/tracking: Tracking Service, tracking client APIs, autologgingarea/models: MLmodel format, model serialization/deserialization, flavorsarea/model-registry: Model Registry service, APIs, and the fluent client calls for Model Registryarea/scoring: MLflow Model server, model deployment tools, Spark UDFsarea/evaluation: MLflow Model evaluation features, evaluation metrics, and evaluation workflowsarea/gateway: MLflow AI Gateway client APIs, server, and third-party integrationsarea/prompts: MLflow prompt engineering features, prompt templates, and prompt managementarea/tracing: MLflow Tracing features, tracing APIs, and LLM integrationsarea/projects: MLproject format, project running backendsarea/uiux: Front-end, user experience, plotting, JavaScript, JavaScript dev serverarea/build: Build and test infrastructure for MLflowarea/docs: MLflow documentation pagesHow should the PR be classified in the release notes? Choose one:
rn/none- No description will be included. The PR will be mentioned only by the PR number in the "Small Bugfixes and Documentation Updates" sectionrn/breaking-change- The PR will be mentioned in the "Breaking Changes" sectionrn/feature- A new user-facing MLflow feature worth mentioning in the release notesrn/bug-fix- A user-facing bug fix worth mentioning in the release notesrn/documentation- A user-facing documentation change worth mentioning in the release notesIs this PR a critical bugfix or security fix that should go into the next patch release?