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

[IMP] Added Dockerfile for wkhtmltox v0.12.6-1 #18

Conversation

alexandregaldeano
Copy link
Contributor

Compared to the other Dockerfiles:

  1. I use Ubuntu 20.04 instead of 18.04
  2. I changed how the deb file is installed (dpkg -i + apt -f install)
  3. I added the minimum number of dependencies such that rendering is exactly the same as in Odoo.sh
  4. I added the --platform flag to avoid pulling the arm64 image which is incompatible with the deb file
  5. I Fixed some warning at image build time

I also added the compose file to go along with it.

This Dockerfile is also useable on Apple Silicon. Note that I first tried to make a Dockerfile for arm64, but the pdf rendering was very flacky.

@sbidoul
Copy link
Member

sbidoul commented Nov 29, 2024

Looks good overall, thanks!

Could you

  • group all the RUN commands for installation in one, for a more compact image, like it is done in the other Dockerfiles?
  • update the GitHub workflow
  • remove the docker-compose part from this PR? I'd rather discuss that independently as I'm not convinced of the added value for this project

@alexandregaldeano alexandregaldeano force-pushed the agaldeano/add-0-12-6-1-wkhtmltox-dockerfile branch 2 times, most recently from 57d7aef to b2a4482 Compare November 29, 2024 09:29
@alexandregaldeano
Copy link
Contributor Author

Done, I'll create the PR for the docker-compose.yml file later. I also changed the image version number in docker run command. Thanks for all the inputs!

Dockerfile-0.12.6.1 Outdated Show resolved Hide resolved
Dockerfile-0.12.6.1 Outdated Show resolved Hide resolved
@alexandregaldeano alexandregaldeano force-pushed the agaldeano/add-0-12-6-1-wkhtmltox-dockerfile branch 3 times, most recently from 6b4fc1e to bad4b04 Compare November 29, 2024 16:16
@alexandregaldeano
Copy link
Contributor Author

I know realized that I forgot to add the 0.12.6.1 version to the tests. Now, it's done :)

@sbidoul
Copy link
Member

sbidoul commented Nov 29, 2024

I know realized that I forgot to add the 0.12.6.1 version to the tests. Now, it's done :)

Yeah, this build and test in separate files is not optimal :/

@alexandregaldeano alexandregaldeano force-pushed the agaldeano/add-0-12-6-1-wkhtmltox-dockerfile branch from bad4b04 to 3f4cad5 Compare November 29, 2024 16:51
@alexandregaldeano
Copy link
Contributor Author

I kept the Focal version on a branch on our repo, we won't use it, but if someone realizes the Jammy version ends-up affecting the rendering vs Odoo.sh, it will be there: https://github.com/foodles-tech/kwkhtmltopdf/tree/agaldeano/add-0-12-6-1-wkhtmltox-dockerfile-focal

@sbidoul sbidoul merged commit bb4efb9 into acsone:master Nov 29, 2024
0 of 3 checks passed
@alexandregaldeano alexandregaldeano deleted the agaldeano/add-0-12-6-1-wkhtmltox-dockerfile branch November 29, 2024 17:11
@sbidoul
Copy link
Member

sbidoul commented Nov 29, 2024

Thanks for this contrib @alexandregaldeano

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants