Updated docker documentation, added .dockerignore #111
Conversation
.dockerignore is always important to have similarly to .gitignore to keep in this case the docker context free from unecessary bloat. Updated the documentation to correct the docker build and run invocation so that it keep the existing style of using a subdirectory to store the dockerfiles but puts the context in the current parent directory so the files can be copied in properly.
|
related to: #72 |
There was a problem hiding this comment.
Thanks @DreamingRaven for the PR! I just left a comment regarding .dockerignore, then we can get it merged.
.dockerignore
Outdated
| tutorials | ||
| third_party |
There was a problem hiding this comment.
I don't think those should be ignored, someone might want to run the docker image to test the tutorials which he expect to be there, third_party libraries also doesn't need to be downloaded if they are already on the host machine. Some example directories that might need to be ignored are build, bin, dist, lib, and wheels which might be generated in the working directory when installing and building the library.
There was a problem hiding this comment.
Ah ok yeah, I misunderstood what third party was, I will remove that line.
As for tutorials, they are data, image, and jupyter notebook files from the looks of it. However they have no way to run jupyter files in your docker containers, nor does the container expose a port for users to interact with the jupyter server if it was running, so they could play around with these tutorials even if the files were included.
I will remove that line if you still want the files in, but I don't think its reasonable to expect an average user to be able to modify the docker container to be able to run jupyter notebooks since the container does not support it, and they are unnecessary for everyone else to my understanding. Although if you want I will fix those issues so users can interact with the jupyter server at a later date if you like.
There was a problem hiding this comment.
True that we doesn't install juputer-notebook in the current images, maybe we should ignore them and build a specific image for that. Then we can leave it as is, but can you add [build, bin, dist, lib, wheels] also, those are the one I know about which are generated when I'm building and playing with the package, I don't want them to be included in the docker image.
There was a problem hiding this comment.
Yeah no problem will add them now.
There was a problem hiding this comment.
also, please merge master into your branch so that I can merge it back to master
[build, bin, dist, lib, wheels]
* Updated docker documentation, added .dockerignore .dockerignore is always important to have similarly to .gitignore to keep in this case the docker context free from unecessary bloat. Updated the documentation to correct the docker build and run invocation so that it keep the existing style of using a subdirectory to store the dockerfiles but puts the context in the current parent directory so the files can be copied in properly. * Changed docker build documentation option order * Re-enabled docker third-party local copy * .dockerignoring locally built tenseal files [build, bin, dist, lib, wheels]
Description
TLDR: Did not change much, just changed documentation to run docker properly and added .dockerignore for good practice.
.dockerignore is always important to have similarly to .gitignore to
keep in this case the docker context free from unecessary bloat.
Updated the documentation to correct the docker build and run invocation
so that it keep the existing style of using a subdirectory to store the
dockerfiles but puts the context in the current parent directory so the
files can be copied in properly.
Affected Dependencies
None
How has this been tested?
ran the new commands
and
please run the above two commands or:
Checklist