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

WIP: Test codespaces #25

Closed
wants to merge 13 commits into from
Closed

WIP: Test codespaces #25

wants to merge 13 commits into from

Conversation

StevenMaude
Copy link
Contributor

Depends on #13.

May not actually be viable, but let's try.

In GitHub Actions workflow.
Using `pigz` cuts the compression time from about 4 minutes to 2 minutes
in the default 4 core GitHub Actions runner.

The `upload-artifact` action also compresses with `zlib` which is
redundant, so let's disable this to speed up the upload step.
One-off comparison in GitHub Actions with the default 4 core runner:

* default `pigz`: 1.7G file size, 1m 49s
* `--fast`: 1.9G file size, 1m 16s

The upload and download time for the file compressed with default
settings is about 10 seconds (turning compression off for the upload).
This is a separate job so that we can easily and cleanly have the
research-template repository checked out.

We could consider moving the smoke test here, although it's maybe
advantageous to have that fail early, before the Docker image is
compressed (because that's a slow step).
Check these are the same as the OpenSAFELY Python Docker image.
It needs to match that which we tag locally, otherwise we'll end up
pulling the image instead of using the locally built image.
There may be better software out there to do this,
but this is available in Ubuntu's repositories,
without having to manually download a software release.

Hopefully, `jq` eventually gets functionality to handle this without
needing another package.
As the comments explain, the R packaging situation is slightly
complicated.

We have additional packages due to the Rocker image. Some of these are
duplicates of those in the OpenSAFELY R image; some with the same
version and some with a different version.

An alternative approach might have been to:

* copy the OpenSAFELY R packages to a path that is unique
* add that library tree to the front of `.libPaths`,
  so these packages always take precedence over other installed versions
* when comparing, compare the `installed_packages` in
  the specific OpenSAFELY library tree of interest:

  ```r
  installed.packages(lib.loc="/usr/local/lib/R/opensafely-site-library")
  ```

  taken from the OpenSAFELY image. That would remove the need for the
  exemptions when checking.

Fow now, it's possibly more useful still being aware of these additional
packages and checking for them explicitly.
With the R checks.
@StevenMaude
Copy link
Contributor Author

We're not going to do this right now. See opensafely-core/codespaces-initiative#18.

@StevenMaude StevenMaude deleted the steve/test-codespaces branch May 23, 2024 17:54
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.

None yet

1 participant