ci: warm-up docker images #2099
Conversation
|
That's roughly 4/5 minutes less in the GHA build on x86_64, doesn't change a thing on qemu builds except for aarch64 where it's slower (probably because graalpy also gets installed). |
|
How much slower? Seems like an overall win? |
|
It seems like an overall win, I will do a proper sum-up this week-end once a couple things are fixed like the emulation run on GHA. |
mayeut
force-pushed
the
docker-warmup
branch
from
5f25a79 to
0552ca1
Compare
|
Here's a sum-up, with last commits:
|
test/conftest.py
Outdated
| if exit_code == "0": | ||
| subprocess.run( | ||
| ["docker", "commit", container_id, image], check=True, stdout=subprocess.DEVNULL | ||
| ) |
There was a problem hiding this comment.
Should we print an error or fail if the command doesn't succeed?
There was a problem hiding this comment.
Good point. should we print or fail ?
Maybe we should just fail here. I will add a commit doing that (& revert if we decide not to fail but print).
test/conftest.py
Outdated
| if worker_id == "master": | ||
| # not executing with multiple workers | ||
| return docker_warmup(request) | ||
|
|
There was a problem hiding this comment.
Is this early return needed? I can't see the downside of the locking in the single-worker case?
There was a problem hiding this comment.
This was taken from https://pytest-xdist.readthedocs.io/en/stable/how-to.html#making-session-scoped-fixtures-execute-only-once
I asked myself the same question but did not add a comment.
I can't see the downside of the locking but I can see one in writing to tmp_path_factory.getbasetemp().parent which might not be so temporary anymore in single worker mode.
| subprocess.run( | ||
| ["docker", "commit", container_id, image], check=True, stdout=subprocess.DEVNULL | ||
| ) | ||
| subprocess.run(["docker", "rm", container_id], check=True, stdout=subprocess.DEVNULL) |
There was a problem hiding this comment.
Might be polite to run this in a finally: block
There was a problem hiding this comment.
Done in the commit that enforce the warm-up does not fail
As it's only done in CI, being polite is not required but when considering failure now, it's easy enough to change this (would require nesting 2 try/except/finally in the commit before that)
Let's see if we can speed-up linux CI by warming-up docker images.
This helps locally but I have a very slow Internet connection so it might not apply here.