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

LORIS-MRI tooling configuration (Docker version) #1182

Closed

Conversation

maximemulder
Copy link
Contributor

@maximemulder maximemulder commented Sep 9, 2024

Alternative to #1170. The differences are the following :

  • In LORIS-MRI tooling configuration #1170, each static test is ran in a GitHub action runner, which builds the Python environment with the PIP dependencies being cached.
  • In this PR, a first GitHub action will build the Python environment in a Docker image, which is cached in the GitHub Container Registry. Then, each static test is ran in this image on their GitHub action runner.

The workflow in this PR is arguably more complex than #1170, but it provides a better isolation and reproducibility for the testing environment. Although this PR caches the whole Docker image instead of just the dependencies, the performance looks similar, as the time gained in the environment setup is lost in launching and stopping the container.

@maximemulder maximemulder force-pushed the 2024-09-09_try-python-docker branch 2 times, most recently from 9b872f0 to 72ac4ee Compare September 9, 2024 17:24
@laemtl laemtl added the Area: CI PR or issue related to continuous integration, including automated tests and static checks label Sep 13, 2024
@maximemulder maximemulder force-pushed the 2024-09-09_try-python-docker branch from 642f6a0 to f779a34 Compare September 13, 2024 15:37
@maximemulder maximemulder force-pushed the 2024-09-09_try-python-docker branch from 60c9bef to 9427c02 Compare September 16, 2024 13:57
@maximemulder
Copy link
Contributor Author

Okay, ignoring the messy Git history (kinda too lazy to fix it now) this PR is done. Although this approach has some advantages, it is also slower than the current Python tests setup, so I am not sure if we should merge it. Anyway, I learned quite a bit about Docker and GitHub Actions which will be useful for integration tests. This PR in itself will be a subject of discussion for another meeting.

@maximemulder
Copy link
Contributor Author

We concluded in the meeting that the dockerization of our Python checks is not worth the complexity and speed hit (~2min) of this PR. Closing.

@cmadjar cmadjar added this to the NA milestone Feb 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: CI PR or issue related to continuous integration, including automated tests and static checks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants