Revise testing infrastructure to decrease spurious failures #759
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| name: Docker Image CI | ||
eitsupi marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| on: | ||
| workflow_dispatch: null | ||
| push: | ||
| paths: | ||
| - tests/ml-test.Dockerfile | ||
| - .github/workflows/cuda-test.yml | ||
|
There was a problem hiding this comment. walk me through your thinking? if we change the workflow or the test, we should run the workflow, right? or is the former implicit anyway? There was a problem hiding this comment. Like this: rocker-versioned2/.github/workflows/scripts-test.yml Lines 3 to 14 in 26c50e5
There was a problem hiding this comment. oh thanks for explaining, got it! done now. |
||
| jobs: | ||
| build: | ||
| runs-on: ubuntu-latest | ||
| permissions: write-all | ||
|
There was a problem hiding this comment. good question, let's try without that. There was a problem hiding this comment. Sorry. What I was trying to say was that read permission would be sufficient. There was a problem hiding this comment. But I don't think this test is ever using the GITHUB_TOKEN here anyway? I think I had left write-all in by mistake from a previous action that was also pushing to github (and even then it should plausibly have a more scoped write permission). I think the test is happy without this. There was a problem hiding this comment. Since this repository is old, I assume write-all privileges are granted by default, but if you follow best practices, you should grant only minimal privileges. There was a problem hiding this comment. oh good to know! I didn't realize they did legacy privileges but of course that makes sense or actions would break. I've gone with read-all now. |
||
| steps: | ||
| - uses: actions/checkout@v3 | ||
| - name: Build the Docker image | ||
| run: docker build -f tests/ml-test.Dockerfile . | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| FROM rocker/cuda:4.3.2 | ||
|
|
||
| ENV S6_VERSION=v2.1.0.2 | ||
| ENV RSTUDIO_VERSION=2023.12.0+369 | ||
| ENV DEFAULT_USER=rstudio | ||
| ENV PANDOC_VERSION=default | ||
| ENV QUARTO_VERSION=default | ||
| ENV PATH=${VIRTUAL_ENV}/bin:${PATH} | ||
|
|
||
| RUN /rocker_scripts/install_rstudio.sh | ||
| RUN /rocker_scripts/install_pandoc.sh | ||
| RUN /rocker_scripts/install_quarto.sh | ||
| RUN /rocker_scripts/install_tidyverse.sh | ||
|
|
||
| #COPY scripts/install_ml-cuda.sh /rocker_scripts/install_ml-cuda.sh | ||
| #RUN /rocker_scripts/install_ml-cuda.sh | ||
cboettig marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| EXPOSE 8787 | ||
|
|
||
| CMD ["/init"] | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,8 +16,7 @@ | |
| "install_geospatial.sh", | ||
| "install_python.sh", | ||
| "install_jupyter.sh", | ||
| "install_julia.sh" | ||
| ], | ||
| "script_arg": [ | ||
| "none" | ||
|
|
@@ -33,7 +32,7 @@ | |
| "base_image": "rocker/r-ver", | ||
| "tag": "devel", | ||
| "script_name": "install_rstudio.sh", | ||
| "script_arg": "latest" | ||
|
There was a problem hiding this comment. As you know, daily has been failing for a week due the RStudio server throwing a 500 error on that download. Because we try to build that image daily in the cron tasks anyway, I think it's less important to also test it here, especially when it means that the current situation ends up blocking the ability to fix anything. Maybe you can provide your preferred solution? There was a problem hiding this comment. Oh, I'm sorry. I didn't know that the daily build was failing. Indeed, as you say, this is used every day, so we don't have to dare to test it here. I'm sure that There was a problem hiding this comment. ok thanks. yeah sorry I get the emails every day when the daily fails (https://github.com/rocker-org/rocker-versioned2/actions/workflows/devel.yml) which happens all the time. It looked like this test was building rstudio on r-ver:devel while other tests were building it on the normal r-ver so I didn't delete it, but if you're fine dropping this test then so am I! |
||
| }, | ||
| { | ||
| "base_image": "rocker/r-ver", | ||
|
|
@@ -53,30 +52,12 @@ | |
| "script_name": "install_quarto.sh", | ||
| "script_arg": "prerelease" | ||
| }, | ||
eitsupi marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| { | ||
| "base_image": "rocker/r-ver", | ||
| "tag": "4.0.4", | ||
| "script_name": "install_tensorflow.sh", | ||
| "script_arg": "none" | ||
| }, | ||
| { | ||
| "base_image": "debian", | ||
| "tag": "stable-slim", | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any reason why we should keep this in a separate yml file?
Is splitting another job not enough?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, not entirely sure I follow. I think having a separate script is necessary to run on another runner? I find this architectural design easier to understand and maintain -- it's fewer pieces and distinct parts are isolated in different files. By splitting another job you mean doing this through the matrix.json file? I did look at the design of a matrix.json for this (in a separate yaml file, though perhaps could be merged): https://github.com/rocker-org/rocker-versioned2/pull/759/files#diff-454bb856f3821991ff2015ec5ba81c69df2ae3b5a476e30104d697c420b35093, and it hits the same issue with space.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My suggestion is simply that it is impossible to move the job to scripts-test.yml.
I don't know if they're sharing capacity on a per-job basis or on a per-workflow basis, but I think it's unlikely that they're sharing it across workflows.
So you probably don't need to split the workflow for a single job here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh sorry to be dense, I think I understand. I've always tended to write actions with basically one job per yaml, unless there were two dependent jobs (e.g. like
generate_matrixandbuildscripts-test.yml). I think I do the same thing with, e.g. writing R functions in many different files inR/. I see the cuda tests as a bit of a work in progress, while for the momentscripts-test.ymlhas been pretty stable, so conceptually to me anyway this provides a bit more separation between the component I'm still intending to fiddle with and the component that works. Why have all the jobs in the same yaml file?