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

Add tests for dev containers #123

Closed
wants to merge 12 commits into from

Conversation

StevenMaude
Copy link
Contributor

No description provided.

lucyb and others added 10 commits April 24, 2024 09:56
This image is based on Tom Palmer's rebuild of the r-docker image, used in the
OpenSAFELY pipeline. It provides the same version of R and copies over all
required packages. It also packages RStudio for use with devcontainers.

The image also provides the version of Python used by the OpenSAFELY pipeline
and copies over the required Python packages.

The ehrql library isn't compatible with this version of Python, so isn't
installed.

Co-authored-by: Jon Massey <[email protected]>
This uses the standard Rocker image and saves use over 3 gig in space (see [comment](opensafely-core#9 (comment))).

It also moves the renv setup into a separate script, where we also setup the
rstudio environment.
The ehrql extension is downloaded and referenced in vs code separately rather
than being installed using pip because it requires a newer version of python
(3.11 rather than the 3.10 required for the analysis code).

Co-authored-by: Jon Massey <[email protected]>
This is so we can use the packages copied over from the python action image.

Co-authored-by: Jon Massey <[email protected]>
The python symlinks included in the virtualenv copied from the Python Action
image point to /usr/bin/python3, so we need to ensure that points to the
correct version of python.
For this initial version, I build the image locally and uploaded it using these
[instructions](https://docs.github.com/en/packages/working-with-a-github-packages-registry/working-with-the-container-registry).
Ubuntu PPA domains changed in 2022; although the old ones were supposed to
remain indefinitely, they're currently inaccessible; but we should update to
the new one anyway

https://blog.launchpad.net/ppa/new-domain-names-for-ppas
This matches the behaviour of GitHub Desktop and is likely to be the behaviour
that users expect. If this causes any problems we can disable it again.
and remove vestigial config added for gitpod
@StevenMaude StevenMaude force-pushed the steve/test-devcontainers branch 3 times, most recently from 84f6d2d to 8c646bd Compare April 30, 2024 16:40
@StevenMaude StevenMaude force-pushed the steve/test-devcontainers branch 16 times, most recently from f10b2e3 to aa5acaa Compare April 30, 2024 17:12
@lucyb
Copy link
Contributor

lucyb commented May 2, 2024

It would be worth considering moving this into the research-template-docker repo to avoid too much cruft in this repo. We'll want to add things like alerts into slack, which will add extra things that we probably don't want/need here. We already have a very basic test of the dev container docker image there, so this builds on that work.

It looks like the checkout action supports pulling in other repos, so I think it should work.

- name: Checkout tools repo
  uses: actions/checkout@v4
  with:
    repository: opensafely/research-template

@lucyb
Copy link
Contributor

lucyb commented May 2, 2024

It's looking like building against macos is a bit more complicated than expected. For now, I would focus on getting it working against ubuntu:latest, which is presumably what GitHub use for Codespaces.

Apologies if you've already thought about this, but I wanted to respond while I had some spare time.

@StevenMaude
Copy link
Contributor Author

For now, I would focus on getting it working against ubuntu:latest, which is presumably what GitHub use for Codespaces.

I'd already dropped macOS as an idea.

Essentially:

  • for Ubuntu, the testing worked first time
  • for macOS, Docker is not provided by default, and requires some setup; this should be possible, but is a little tricky to get right (there are third-party actions that do this, but I was trying to include a minimal set of configuration that works)
  • for Windows, Docker is pre-installed, there are issues around getting Buildx for Docker installed, which is required for Docker Buildkit; it might work at some point in the future

It would be worth considering moving this into the research-template-docker repo to avoid too much cruft in this repo.

That's fine; I'll do that 😎

@Jongmassey Jongmassey force-pushed the improve-devcontainers branch 3 times, most recently from 2295841 to 19509fb Compare May 3, 2024 13:45
@StevenMaude StevenMaude closed this May 3, 2024
@StevenMaude StevenMaude deleted the steve/test-devcontainers branch May 8, 2024 15:30
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.

3 participants