Use self-hosted runner #84
Conversation
.github/workflows/ci.yml
Outdated
| - name: Download test data | ||
| env: | ||
| TEST_DATA_PASSWORD: ${{ secrets.test_data_password }} | ||
| run: | | ||
| export TESTDATA_MD5SUM=`grep 'TESTDATA_MD5SUM' ./tests/conftest.py | awk -F"'" '{print $2}' | tr -d '\n'` | ||
| wget --post-data "pass=$TEST_DATA_PASSWORD" "https://vhega.iaa.es/iop4/iop4testdata.tar.gz?md5sum=$TESTDATA_MD5SUM" -O $HOME/iop4testdata.tar.gz |
There was a problem hiding this comment.
We could even avoid this step in the self-hosted runner, no?
There was a problem hiding this comment.
Yes, we can provide the test dataset to the runners the same way as the astrometry index files.
There was a problem hiding this comment.
We could even avoid this step in the self-hosted runner, no?
By providing the test dataset we could save ~1min of download, and if we provided it already uncompressed, ~2min more (~3min total). But this would require modifying the tests themselves, since the unpacking is done there. We can do this on a different PR.
There was a problem hiding this comment.
+1, this can be left untouched
This happens inside the runner. We might increase a bit the host memory or reduce the parallelization in the tests. Maybe this was the reason it was not working in the github runners when the astrometry index files were correctly mounted. |
Do you have an estimate on how much memory was used? You could reduce the a bit parallelization |
Almost 2GB. The host had 2GB, so the runner can't use 2GB fully, and the OOM killed it when it tried to use more. |
|
👏 👏 👏 |
Weird, now it is the host killing the lxc containers. Let me investigate. |
| @@ -22,6 +22,7 @@ env: | |||
|
|
|||
|
|
|||
| jobs: | |||
|
|
|||
| static-code-checks: | |||
There was a problem hiding this comment.
currently the static checks step does nothing but setting up python, it could be used for formatting, lining (unrelated to this PR)
.github/workflows/ci.yml
Outdated
| - name: Download test data | ||
| env: | ||
| TEST_DATA_PASSWORD: ${{ secrets.test_data_password }} | ||
| run: | | ||
| export TESTDATA_MD5SUM=`grep 'TESTDATA_MD5SUM' ./tests/conftest.py | awk -F"'" '{print $2}' | tr -d '\n'` | ||
| wget --post-data "pass=$TEST_DATA_PASSWORD" "https://vhega.iaa.es/iop4/iop4testdata.tar.gz?md5sum=$TESTDATA_MD5SUM" -O $HOME/iop4testdata.tar.gz |
There was a problem hiding this comment.
+1, this can be left untouched
|
Tests passing, I think it can be merged now. |
This PR changes the CI to use the self-hosted runners provided by the VHEGA group.
The self-hosted runners are ephemeral and provisioned on demand, just like the GitHub-hosted runners, but provide the astrometry index files, removing the need of downloading them every time. This was the main reason why tests were previously failing. They also provide slightly increased RAM and CPU, making tests faster.
This solves #19, finally and probably once and for all.