Mount local artifact folder to docker container in MLProject

[nlaille]

What changes are proposed in this pull request?

Fix issue : #1450

Mount artifact folder to docker container if artifact_uri is local.

How is this patch tested?

Add a new unit test in tests/projects/test_docker_projects.py

Manual test with examples/docker

Release Notes

Is this a user-facing change?

• No. You can skip the rest of this section.
• Yes. Give a description of this change to be included in the release notes for MLflow users.

Fix issue on logging artifact using local file system as artifact storage and docker MLProject.

What component(s) does this PR affect?

• UI
• CLI
• API
• REST-API
• Examples
• Docs
• Tracking
• Projects
• Artifacts
• Models
• Scoring
• Serving
• R
• Java
• Python

How should the PR be classified in the release notes? Choose one:

• rn/breaking-change - The PR will be mentioned in the "Breaking Changes" section
• rn/none - No description will be included. The PR will be mentioned only by the PR number in the "Small Bugfixes and Documentation Updates" section
• rn/feature - A new user-facing feature worth mentioning in the release notes
• rn/bug-fix - A user-facing bug fix worth mentioning in the release notes
• rn/documentation - A user-facing documentation change worth mentioning in the release notes

[mateiz]

This is a good catch but unfortunately it doesn't merge cleanly anymore. Do you mind updating it to fix the merge conflicts? Hopefully it won't need a large change. Thanks for adding a test too!

[nlaille]

@mateiz done.
Btw an extension to this one : #1621

[smurching]

Thanks @nlaille for the PR! Just to make sure things work end-to-end, would you mind:

1. Adding logic to this example docker project script that logs an artifact, and
2. Updating this test to verify that the artifact was indeed logged? You should be able to use the MlflowClient's list_artifacts API (see docs, an MLflowClient object is already available in the test as mlflow_service) to verify that the artifact is present.

[smurching]

To clarify, the PR otherwise LGTM - thanks!

[nlaille]

@smurching I added a way to handle relative path as well. Luckily the test highlighted it :)

[codecov-io]

Codecov Report

Merging #1544 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1544   +/-   ##
=======================================
  Coverage   83.66%   83.66%           
=======================================
  Files          20       20           
  Lines        1120     1120           
=======================================
  Hits          937      937           
  Misses        183      183

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 34b3b18...3830487. Read the comment docs.

[smurching]

Would be nice to pull "/mlflow/projects/code/" into a constant so we don't have this string duplicated across mlflow/projects/__init__.py, if you want to make a follow-up PR to do that :)

[smurching]

LGTM thanks @nlaille for the updates!

[leobenkel]

Hello,
I am still having an issue on this topic.
I have one container1 that hold MLFLow server.
And one container2 that hold Jupyter notebook.
When the container Container2 runs mlflow.sklearn.log_model, the model is being saved locally under ./tmp/mlflow instead of in the container Container1.

Both of them have a mounted volume for the artifact folder but I can't find how to set up a default path in the client.

[sucitw]

@leobenkel,

I had the same issue to access artifacts when running MLflow server via container. I fixed it by the "docker volume" command.

According to this stackoverflow

The best practice for artifact storage with a remote tracking server is to configure the server to use an artifact root accessible to both clients and the server.

Therefore, for the container approach, you need to use docker volume to mount the artifact folder for each container. Then, you are supposed to see artifacts from the client and server-side.

[leobenkel]

Thank you @sucitw !