-
Notifications
You must be signed in to change notification settings - Fork 17
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.
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
Docker image for running generators #5
base: main
Are you sure you want to change the base?
Conversation
Option for building docker image that runs generators
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice idea!
|
||
# && rm -rf /var/lib/apt/lists/* | ||
# Not sure I need libc6-i386 | ||
# g*-multilib is an overkill but will be useful for compiling |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In which scenarios do you think 32-bit compilations will be useful? I think we can remove them for simplicity.
# g*-multilib is an overkill but will be useful for compiling | ||
|
||
RUN update-alternatives --install /usr/bin/python python /usr/bin/python3.6 1 && \ | ||
/usr/bin/python3 -m pip install --upgrade pip && \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haven't we updated pip already above?
# Not sure I need libc6-i386 | ||
# g*-multilib is an overkill but will be useful for compiling | ||
|
||
RUN update-alternatives --install /usr/bin/python python /usr/bin/python3.6 1 && \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this for making python
point to python3
? If so, we could instead install the python-is-python3
package.
|
||
# Install OpenJDK-11 | ||
RUN apt-get update && \ | ||
apt-get install -y openjdk-11-jre-headless && \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why use a separate apt-get call for JDK?
@@ -0,0 +1,61 @@ | |||
FROM ubuntu:18.04 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use 20.04?
@@ -9,6 +9,10 @@ Planning Competitions (IPC). | |||
* Build single generator: ``cd assembly; make`` | |||
* Test generators: ``sudo apt install python-tox && tox`` | |||
|
|||
## Docker instructions | |||
* Build in Docker image: ``docker build -t pddl-generators .`` | |||
* Run Docker container: ``docker run -ti -t pddl-generators`` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Double -t
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be good to use long option names for Docker beginners: --tag, --tty, --interactive
|
||
COPY . . | ||
|
||
RUN ./build_all save-logs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you save the build logs? Couldn't they confuse users who just want to run the generators? You'll see the build output when the container is built, won't you?
My immediate reaction is "why do this when it's meant to be just placed inside of You game for that, @hectorpal ? It would mean that from a |
Good point. It'd be better to have it in planutils. I'll retract the PR and create one in planutils. |
If I understand Christian correctly, he suggests to keep the Dockerfile in this repo and then use it for adding the PDDL generators to planutils. I think this makes sense. |
That is indeed what I had in mind. Just need to wrap the completed docker into a singularity so that the generators can be used. This related at all to the changes @guicho271828 is running with over on #6 ? |
|
#6 is a simple attempt to make pddl-generators a unified unix-like tool. |
It's convenient for some people.