Add an edit-mode option

[evertrol]

This adds an option to run from a local repository in edit mode, where
changes in a running Docker container (for example, through a
notebook) are reflected in the local repository.

Implements the feature suggested in #357.

[evertrol]

This may still require some tests, which are probably quite a bit more work than implementing the actual feature (given that changes in both a local directory and a container need to be verified).

[betatim]

Test will indeed require a bit of thought. Some ideas below.

What do you think of using the tests from #413 (https://github.com/jupyter/repo2docker/pull/413/files#diff-73b9b926dafec45bd12128b598a0dd45) as a starting point?

I was thinking if we can execute repo2docker from the "outside" (with most of the other tests we only get to run code in verify inside the container) then we can use a start file that is executed inside the container to make a change and check that the local directory was indeed changed.

[betatim]

In terms of things we need:

• tests
• documentation in form of a short "How to ..." section

(if you want to you can move this list to the top comment, not sure if GitHub let's you tick boxes in my comment)

[evertrol]

I've put the documentation as a FAQ (even if it isn't (yet)), since I'm not sure how well it fits as a how-to. And I didn't see documentation on the individual options (other than what argparse produces on the command line, obviously).

Also: I can't tick the check-box :-(

[evertrol]

I seem to have hit a little snag: I can't bind an external volume to an internal volume using an internal variable. I'd like to bind to $HOME or ~, but that just get escaped inside client.containers.run(..., volumes=container_volumes, ...). I can get hardcode /home/jovyan for tests, but that is obviously not a solution when using user-supplied Docker files.

I've been looking at another way to work around this, or find a configuration option to change this, but no luck so far. Would anyone know how to do this?

(to be clear, I'd like to do:

client.containers.run(..., volumes={'/host/directory', {'bind': '$HOME', 'mode': 'rw'}}, ...)

)

[betatim]

I think we could resolve $HOME before making that call?

I'd be Ok with having the docs say "Editable mode is a convenience option that will mount the repository to /home/$USER, if you need to mount to a different location in the container please use --volumes." if we can't find a solution to this.

[evertrol]

I have yet to figure out how to resolve $HOME before the container is run (or, otherwise, mount a volume afterwards).

But it looks like using self.volumes[os.path.abspath(args.repo)] = '.' actually works, because the working directory is picked up from the image and prepended in case of a relative directory:

            image_workdir = image['ContainerConfig']['WorkingDir']

            for k, v in self.volumes.items():
                container_volumes[os.path.abspath(k)] = {
                    'bind': v if v.startswith('/') else os.path.join(image_workdir, v),
                    'mode': 'rw'
                }

And the working directory generally appears to be set to $HOME:

WORKDIR ${HOME}

If we can assume this will always be the case for repo2docker containers, than the code as it is currently, works, and there is no need to resolve $HOME.

There may have to be a caveat pointing out this doesn't work for fully customized containers(*), similar to the caveat about mounting to another location in the container (the two caveats would probably go together).

(*) I don't actually know if that's true: can a user's Dockerfile remove/ignore the working directory setting? The failing test case as described below suggests it can.

---

The thing that bugs me, and led me on this chase to $HOME, is that a simple test based on the dockerfile/simple test, but with ['-E', 'testdir'] added to the arguments, fails: in this case, image['ContainerConfig']['WorkingDir'] is empty.

To resolve that issue, I could

• try and tweak the test
• add a WORKDIR ${HOME} to the Dockerfile used in the test, as in the base Dockerfile image.
• something else?

Any thoughts on this?

[betatim]

You could inspect image['ContainerConfig']['Env'] to find $HOME. This would make it a bit harder to reason about which directory inside the container will be used as it would be "Relative to WorkingDir if set, otherwise relative to $HOME if set, if neither is set relative paths aren't supported"

I think it is fine to assume that WorkingDir is set and error out if not. The general philosophy for repo2docker is that if you use your own Dockerfile you are an expert and know what you are doing. We offer very little support to you when things don't work beyond "do you really need to make your own Dockerfile?". Conversely we try to support the things a lot of users want to do natively so that they do not have to write a Dockerfile.

So I would keep the current behaviour and add a test that a Dockerfile with WorkingDir set works as expected.

[evertrol]

I've added two tests:

• a test that creates a file inside the container, for which it is checked that it can be viewed by the host (with its contents verified)
• a test that creates a file on the host, while the container is running. The existence of that file is then verified inside the container (though not its contents).

The five-line fixture can be replaced by an import once #413 gets merged.

---

Additionally, I have converted the FAQ from Markdown to reStructuredText, since I wanted an admonition note, which is not possible with Markdown. Perhaps this change should be taken out of this PR and put into its own PR.

Lastly, I was looking for a CHANGES file to add this change, but there isn't any. Perhaps the repository is considered the CHANGES file, but generally, I find a separate file easier to read and spot (important) changes between versions, or bug-fixes.
Perhaps it's something to consider? Though I notice (sadly) few (notable) Python projects actually have such a file.

[betatim]

Having a ChangeLog would be welcome at least by me. We haven't been very disciplined so far so if you oulc get us started that would be great.

Do you have a preferred format? My suggestion would be to start with a style a bit like https://github.com/scikit-learn/scikit-learn/blob/master/doc/whats_new/v0.19.rst#version-0191.

Changing from MD to rst would be ideal in a separate PR so we can keep reformatting separate from content changes.

[evertrol]

@betatim Thanks for all the feedback.

• the faq.md to faq.rst change has been taken out, and is now its own PR
• a suggested change log format is now submitted as a PR for comments

[betatim]

I'll merge this now to get this cool feature in there.

[neuromusic]

sorry for taking so long to say "thanks" but this is awesome! going to test it out ASAP