-
Notifications
You must be signed in to change notification settings - Fork 3
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
Support Podman #197
Support Podman #197
Conversation
I need to dig into the failed tests, apparently caused by the change in the default image. |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #197 +/- ##
==========================================
- Coverage 86.32% 85.86% -0.46%
==========================================
Files 9 9
Lines 914 920 +6
==========================================
+ Hits 789 790 +1
- Misses 125 130 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@@ -78,7 +78,7 @@ def _pull_docker_image(docker_client): | |||
# Avoid interfering with used ports on the host system. | |||
@pytest.fixture(scope="session", autouse=True) | |||
def _default_port(monkeypatch_session): | |||
monkeypatch_session.setattr(aiidalab_launch.profile, "DEFAULT_PORT", None) | |||
monkeypatch_session.setattr(aiidalab_launch.profile, "DEFAULT_PORT", 7777) |
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.
Podman seems to autoassign random port, so the tests were failing for me locally.
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.
Thanks! I have no podman to test, so I approve it.
Leave a minor open question of which registry to use.
@@ -162,7 +173,7 @@ SOFTWARE. | |||
## Acknowledgements | |||
|
|||
This work is supported by the | |||
[MARVEL National Centre for Competency in Research](<http://nccr-marvel.ch>) funded by the [Swiss National Science Foundation](<http://www.snf.ch/en>), | |||
[MARVEL National Centre for Competency in Research](<https://nccr-marvel.ch>) funded by the [Swiss National Science Foundation](<https://www.snf.ch/en>), |
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.
Thanks!
@@ -31,7 +31,9 @@ def _default_port() -> int: # explicit function required to enable test patchin | |||
return DEFAULT_PORT | |||
|
|||
|
|||
DEFAULT_IMAGE = "aiidalab/full-stack:latest" | |||
DEFAULT_REGISTRY = "docker.io" |
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.
docker registry has a quite strict API rate limits. What about using ghcr.io
?
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.
Well, it's currently dockerhub. The API rate limits should not really apply to users of aiidalab-launch
for whom downloading a new AiiDAlab image should be a relatively rare operation.
I think it's better to keep using dockerhub for production images and ghcr.io
for development. If there's a disagreement on that let's discuss separately.
Podman seems to work out-of-a-box with a little extra setup. I've also tweaked the
DEFAULT_IMAGE
to include thedocker.io/
prefix since that might not be default for Podman.Closes #196