make storage directory in Docker storage configurable (fixes #2861) #2865
Conversation
There was a problem hiding this comment.
Thanks @jameslamb, overall this looks good to me. Just a few small cleanups and I'll be happy to merge.
| @@ -515,6 +527,32 @@ def test_create_dockerfile_with_weird_flow_name(): | |||
| ) | |||
|
|
|||
|
|
|||
| def test_create_dockerfile_with_weird_flow_name_custom_prefect_dir(): | |||
| with tempfile.TemporaryDirectory() as tempdir_outside: | |||
There was a problem hiding this comment.
Instead of using tempfile.TemporaryDirectory, I recommend using pytest's tmpdir fixture: https://docs.pytest.org/en/stable/tmpdir.html
(I know tempfile.TemporaryDirectory() is used elsewhere already, we're slowly migrating towards making better use of fixtures as tests are updated)
There was a problem hiding this comment.
oh sure, I can do that. I just copied this from another test and added in the prefect_directory kwarg.
There was a problem hiding this comment.
Ok I just changed the two tests in this PR that were using tempfile.TemporaryDirectory() to tmpdir.
Thanks for showing me that! I think I'll start using it in other projects.
Co-authored-by: Jim Crist-Harif <jcrist@users.noreply.github.com>
changes/directory (if appropriate)docs/outline.tomlfor API reference docs (if appropriate)What does this PR change?
This PR implements my proposal from #2861 to allow users to configure the location inside
Dockerstorage where flows and other files are stored.This PR sets that location to
/opt/prefect(the previous hard-coded value) by default, so it should not break any existing code.Why is this PR important?
See the discussion in #2861 . This argument gives users finer control over the images produced by
Dockerstorage, and is useful when working with images that have a non-rootUSERset which doesn't have permission to write to/opt.