Skip to content

Add docker image #81

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Add docker image #81

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
435eb16
Add docker image
lilianabs Apr 2, 2023
5f766b9
Add script for creating and running docker container
lilianabs Jun 20, 2023
a163d75
Make Python version configurable
lilianabs Jun 28, 2023
b32a8fb
Use local folder
lilianabs Jun 29, 2023
00b6247
Format Dockerfile and shell script
lilianabs Jul 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions docker-image/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
FROM python:3.7
Copy link
Owner

@neomatrix369 neomatrix369 Jun 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See if you can make this python version configurable that you can pass it via the shell script or environment variable.

If you look into the docker shell scripts in any of these examples here, you can see how it's done.

Copy link
Author

@lilianabs lilianabs Jul 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for pointing this out. I've updated the Dockerfile to pass the argument via the shell script.


Copy link
Owner

@neomatrix369 neomatrix369 Jun 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try to get familiar with hadolint and apply it to your Dockerfile to see what suggestions it gives you in terms of improving the code, here's a post to learn more about it

Copy link
Author

@lilianabs lilianabs Jul 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've used hadolint to format the Dockerfile accordingly :)

RUN apt-get update

RUN apt-get install -y cmake

RUN apt-get install -y git

RUN git clone https://github.com/neomatrix369/nlp_profiler.git
Copy link
Owner

@neomatrix369 neomatrix369 Jun 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of doing this try to see if you can actually use the local folder and the code downloaded in it - there is an advantage and purpose for doing it.

See how it has been done here.

Copy link
Author

@lilianabs lilianabs Jul 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the suggestion Mani. I've updated the file so it uses the local folder.


WORKDIR nlp_profiler

RUN pip install -r requirements-dev.txt

RUN pip install -r requirements.txt

RUN pip install -r requirements-nix-dev.txt
20 changes: 20 additions & 0 deletions docker-image/run-nlp-profiler-in-docker.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#!/bin/bash
Copy link
Owner

@neomatrix369 neomatrix369 Jun 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try to get familiar with a bash shell linter to see what comments it has about your shell script - search for shellcheck, install it and then use it on this shell script.

Use the comments from shellcheck to improve the code if any comments are given


set -e
set -u
set -o pipefail

MOUNT_VOLUME="${PWD}/../"
TARGET_FOLDER="/nlp_profiler"
DOCKER_IMAGE_NAME="nlp_profiler"

echo "~~~ Running nlp_profiler in a docker container"
echo "Mounted volume: ${MOUNT_VOLUME}"

Copy link
Owner

@neomatrix369 neomatrix369 Jun 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its a good practise to echo the steps that are executed at the different lines in the code - this way when one reads the console they can tell what stage of the shellscript it is at - look at examples of my own code to find out how we have been doing it, the clue is look for echos in the shell script and also in the Dockerfile

docker build -t ${DOCKER_IMAGE_NAME} .

Copy link
Owner

@neomatrix369 neomatrix369 Jun 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a good practice to show this line executing with all its parameters, how would you do that in bash?

Also look at one of the examples from the resources shared above

docker run -it \
--volume ${MOUNT_VOLUME}:${TARGET_FOLDER} \
--workdir ${TARGET_FOLDER} \
${DOCKER_IMAGE_NAME} \
/bin/bash