Skip to content
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

Small proposed changes for Docker image creation #519

Closed
raducoravu opened this issue Dec 21, 2023 · 2 comments · Fixed by #520
Closed

Small proposed changes for Docker image creation #519

raducoravu opened this issue Dec 21, 2023 · 2 comments · Fixed by #520
Labels
enhancement Changes to an existing topic or feature

Comments

@raducoravu
Copy link
Member

Based on this article:
https://www.dita-ot.org/dev/topics/using-docker-images
I experimented with creating a Docker image for the Oxygen Publishing Engine:
https://blog.oxygenxml.com/topics/creating_a_docker_image_for_the_oxygen_publishing_engine.html

A couple of remarks about the proposed command to run docker in the DITA OT documentation:

$ docker run -it \
  -v /Users/username/source:/src ghcr.io/dita-ot/dita-ot:4.1.2 \
  -i /src/input.ditamap \
  -o /src/out \
  -f html5 -v

Those "-it" flags do not seem useful in this context, also ideally the "--rm" flag would be added to automatically remove the container after it finishes because containers would keep accumulating although they are useless once they produce the output.
The "--name dita-ot-publish" param can also be used to give a human readable name to the container.

I started from the DITA OT Dockerfile (so thanks for having it):
https://github.com/dita-ot/dita-ot/blob/develop/Dockerfile
and I created my equivalent:
https://github.com/oxygenxml/blog/blob/master/build/Dockerfile

I made some simplifications by avoiding things like:

mv /tmp/dita-ot-$VERSION/config /opt/app/config && \
mv /tmp/dita-ot-$VERSION/lib /opt/app/lib && \
mv /tmp/dita-ot-$VERSION/plugins /opt/app/plugins && \
mv /tmp/dita-ot-$VERSION/build.xml /opt/app/build.xml && \
mv /tmp/dita-ot-$VERSION/integrator.xml /opt/app/integrator.xml && \

and just moving the entire dita-ot folder to the "/opt/app" subfolder.

@raducoravu raducoravu added the bug Something isn't working label Dec 21, 2023
@jelovirt jelovirt added enhancement Changes to an existing topic or feature and removed bug Something isn't working labels Dec 21, 2023
jelovirt added a commit that referenced this issue Dec 21, 2023
Remove reduncant -it arguments to docker and add --rm to remove the container after use.

Fixes #519
@jelovirt jelovirt linked a pull request Dec 21, 2023 that will close this issue
@jelovirt jelovirt added this to 4.2 Dec 21, 2023
@jelovirt jelovirt moved this to In Progress in 4.2 Dec 21, 2023
jelovirt added a commit that referenced this issue Dec 21, 2023
Remove reduncant -it arguments to docker and add --rm to remove the container after use.

Fixes #519

Signed-off-by: Jarno Elovirta <[email protected]>
infotexture pushed a commit that referenced this issue Dec 21, 2023
Remove reduncant -it arguments to docker and add --rm to remove the container after use.

Fixes #519

Signed-off-by: Jarno Elovirta <[email protected]>
@github-project-automation github-project-automation bot moved this from In Progress to Done in 4.2 Dec 21, 2023
@infotexture
Copy link
Member

infotexture commented Dec 21, 2023

@raducoravu Thanks for these suggestions, and thanks to @jelovirt for implementing the flag changes. 🙇

The "--name dita-ot-publish" param can also be used to give a human readable name to the container.

I made some simplifications by avoiding things like:

@raducoravu @jelovirt Should we implement those suggestions directly in dita-ot/Dockerfile, or do we need to preserve that as is for backwards compatibility?

@jelovirt
Copy link
Member

IMO the --name doesn't need to be added to the examples, because the container will be removed after it exists.

Some changes in the Oxygen DITA-OT Dockerfile may be worth investigating, but it's a matter of coding style and e.g. the docs are not needed in the Docker image.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Changes to an existing topic or feature
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants