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

chore: update Dockerfile to be based on the Node 20 image #630

Merged
merged 3 commits into from
Aug 1, 2024

Conversation

jkuester
Copy link
Contributor

Description

Now that we are supporting Node 20, we can bump the version of Node used in the Dockerfile. The only impact this had is that the newer Python package is now requiring we install pyxform into a virtual environment.

One solution would be to just run the pip install with the --break-system-packages flag which should be fine in a simple Docker image like this since the whole point of the containerization is that we don't have competing dependency stacks.

That being said, after doing some reading I have instead opted for actually using a virtual environment here. It was not too much trouble and it felt more future-proof (and less janky).

Testing

We do not have any automated testing around the Dockerfile, but you can manually test its functionality with the following steps:

  • Checkout this cht-conf branch locally and cd into the branch
  • Build the docker image: docker build -t medicmobile/cht-app-ide .
  • Follow the example from the docs to just run the docker image as a standalone utility.
    • The most important action to validate is compile-forms since that is what will use the pyxform binaries.

Code review items

  • Readable: Concise, well named, follows the style guide, documented if necessary.
  • Documented: Configuration and user documentation on cht-docs
  • Tested: Unit and/or integration tests where appropriate
  • Backwards compatible: Works with existing data and configuration. Any breaking changes documented in the release notes.

License

The software is provided under AGPL-3.0. Contributions to this project are accepted under the same license.

@jkuester jkuester marked this pull request as ready for review August 1, 2024 14:23
Copy link

@tatilepizs tatilepizs left a comment

Choose a reason for hiding this comment

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

Thank you @jkuester it worked perfectly 🚀

Screen.Recording.2024-08-01.at.8.44.56.AM.mov

@jkuester jkuester changed the title Update Dockerfile to be based on the Node 20 image chore: update Dockerfile to be based on the Node 20 image Aug 1, 2024
@jkuester jkuester merged commit 4c8571e into main Aug 1, 2024
7 checks passed
@jkuester jkuester deleted the node-20-in-dockerfile branch August 1, 2024 14:48
@medic-ci
Copy link
Collaborator

🎉 This PR is included in version 4.1.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

4 participants