python-3 container template's Dockerfile unnecessarily refers to requirements.txt.temp

[songololo]

• Name of Dev Container Definition with Issue: python-3

The python-3 template Dockerfile requires a requirements.txt.temp file.

If this file is not present, then an error is thrown.

It copies the file into the docker build, but does not appear to do anything with this file.

Have I misunderstood the file's purpose, or is this possibly an artefact from earlier versions?

[Chuxel]

The goal is to actually install your requirements from the root folder requirements.txt (up a level) when the Docker image when it is built if one exists. The .temp file is there to handle the scenario where there is no requirements.txt. The intent here is that you drop the .devcontainer folder into your local repository. The requirements.txt in the root folder is in the .npmignore and therefore is not added to your project when you use it via VS Code's Remote-Containers: Create Container Configuration File... or Remote-Containers: Open Folder in Container... / Remote-Containers: Reopen Folder in Container commands if no .devcontainer folder is present.

@qubitron This was also mentioned by @kmehant in #66, so it seems to be creating some confusion... though I don't think there's actually anything functionally wrong.

Now that we have postCreateCommand, we could consider removing this part in favor of a comment in devcontainer.json. The test project could just do it as a pre-launch task.

What do you think?

[qubitron]

@Chuxel would a postCreateCommand run every time the container is created? That seems like a good solution since this is a bit of a hack to work around limitations in the Dockerfile.

[Chuxel]

@qubitron Yep - Its intended for things like this and npm install in the node world. It happens whenever a new container is created (even if an image is reused or referenced directly). The up-side is it won't be cached so you'll get the latest bits each time, the down side is that is at the cost of perf for rebuilds since you don't get the advantage of layer caching.

We could probably do pip install -r requirements.txt || echo Pip error or no requirements.txt found. so the build doesn't fail out when there's no requirements.txt. (Have to test that on both Windows and Linux/macOS, but pretty sure it works.)

[songololo]

The requirements.txt makes sense to me (and the usage thereof from within the Dockerfile seems clear).

What I still don't understand is what the requirements.txt.temp file does? Particularly as it is copied into the docker container; throws an error if not present; but isn't actually used from within the container (unless I'm missing something)?

The reason I'm asking is I'm preparing some instructions for launching a module in vscode via a dev-container and I don't really know what to say about requirements.txt.temp - it adds an extra step to the instructions but doesn't seem to add anything.

Lest I come across as a whiner: Thanks for the remote-dev / dev-container functionality - it is an awesome feature for dev!

[qubitron]

@Chuxel the problem is that the workspace mount isn't available during build time, so there's still the problem of needing to copy in the requirements.txt file in a way that doesn't fail the build if it's not there

@songololo with the COPY command the the docker build will fail if the requirements.txt file isn't in the your workspace. So the requirements.txt.temp file ensures that at least there is always a file available to be copied and the copy command succeeds regardless of whether you have a requirements.txt file in your workspace or not. The wildcard then will copy the actual requirements.txt file if it exists, and then the rm command will remove both of them afterwards so that they don't interfere with the workspace mount.

[qubitron]

@songololo I'll add that the intention is that you copy the entire .devcontainer folder in, or create it using "create container configuration files" command, so you could simplify the steps by starting with the pre-defined containers and then updating the docker file as you see fit.

[Chuxel]

The problem is that the workspace mount isn't available during build time, so there's still the problem of needing to copy in the requirements.txt file in a way that doesn't fail the build if it's not there

@qubitron The good news is that the postCreateCommand is run after the server is provisioned in the running container with the mount. It's the last thing to happen before everything is up.

I just checked, and this will install requirements.txt if found:

"postCreateCommand": "if [ -f requirements.txt ]; then pip install -r requirements.txt; fi",

[Chuxel]

@qubitron I am making a pass through the dev container definitions as a part of #78 to consolidate layers. As a part of that work, I tried moving these to postCreateCommand and it seems to work pretty well.

As I said, the only downside is that the env updates do not get baked into the image for rebuilds.

The miniconda example is probably the best one to illustrate the problem given its installing Jupyter. It's in the layer-consolidation branch here: https://github.com/microsoft/vscode-dev-containers/tree/layer-consolidation/containers/python-3-miniconda

Easy enough to revert - let me know what you htink.

[qubitron]

Was discussing with @brettcannon and he preferred to keep things the way they were, I believe the caching benefits outweigh the complexity of having the additional file.

Perhaps we can drop an explanation in the requirements.txt.temp file instead to explain it's purpose?

[Chuxel]

@qubitron Yep, we can do that -- along with some additional comments inline in the Dockerfile specifically about the temp file.

[Chuxel]

@qubitron @brettcannon Added this to that same branch. I called the file noop.txt and the file itself explains why it exists (instead of requirements.txt.tmp). I also copied the files to a temp location instead of WORKDIR to avoid any confusion about the files being dropped into the source code tree once it is mounted. See what you think.

https://github.com/microsoft/vscode-dev-containers/tree/layer-consolidation/containers/python-3-miniconda

[Chuxel]

Should be resolved in #81.