Skip to content
New issue

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

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

Already on GitHub? # to your account

Replace nix with a devcontainer #556

Merged
merged 41 commits into from
Nov 5, 2024
Merged

Replace nix with a devcontainer #556

merged 41 commits into from
Nov 5, 2024

Conversation

jorg-vr
Copy link
Contributor

@jorg-vr jorg-vr commented Oct 30, 2024

This pr replaces nix and poetry as package management systems with a devcontainer.

The devcontainer builds the production docker image, and then adds git support and development dependencies to it.

For ease of development I moved the production dockerfile to this repo as well. This means that dependencies can be added to it or updated while developing. Merging a pr will build the docker image, making sure the docker image is always up to date with the latest TESTed version of the code.

I also restructured the docker file, such that it is clearer which dependency is required by which language. I hope this improves maintainability.

To avoid duplication nix, poetry and dependencies.md are all removed. The docker image is now the single source of truth for all production package versions. Dev dependencies should be specified in dev-dependencies.sh.

The README is also updated to reflect these changes.

The solution is still not ideal as updating the dependencies is still a manual job and dependabot is not able to work with docker deps. We might consider adding some custom system as proposed here in a later stage: dependabot/dependabot-core#2129 (comment)

@jorg-vr jorg-vr added the chore label Oct 30, 2024
@jorg-vr jorg-vr self-assigned this Oct 30, 2024
@jorg-vr jorg-vr requested review from bmesuere and rien November 4, 2024 12:59
@jorg-vr jorg-vr marked this pull request as ready for review November 4, 2024 12:59
Copy link
Member

@bmesuere bmesuere left a comment

Choose a reason for hiding this comment

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

Do we now need to manually keep this dodona-tested dockerfile in sync with the one in our images repo?

.devcontainer/devcontainer.json Outdated Show resolved Hide resolved
ENV NODE_PATH /usr/lib/node_modules

# Install dependencies
RUN <<EOF
Copy link
Member

Choose a reason for hiding this comment

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

This will create only a single layer in the file system? If so, I think going forward our other images can also adopt this style instead of adding everything to a single command. It is much more readable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes this only creates a single layer. And I approve of this.

Copy link
Member

@rien rien left a comment

Choose a reason for hiding this comment

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

🥲

.devcontainer/devcontainer.json Outdated Show resolved Hide resolved
Co-authored-by: Bart Mesuere <mesuerebart@gmail.com>
@jorg-vr
Copy link
Contributor Author

jorg-vr commented Nov 4, 2024

Do we now need to manually keep this dodona-tested dockerfile in sync with the one in our images repo?

No I will remove the file from our images repo. I have added the build deploy step here, so we only need the file in a single location

@jorg-vr jorg-vr requested a review from bmesuere November 4, 2024 13:51
@jorg-vr jorg-vr merged commit d2b8963 into master Nov 5, 2024
9 checks passed
@jorg-vr jorg-vr deleted the chore/devcontainer branch November 5, 2024 14:18
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants