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

Trigger pull explictly if needed #184

Merged
merged 2 commits into from
Jul 5, 2023
Merged

Conversation

unkcpz
Copy link
Member

@unkcpz unkcpz commented Jul 3, 2023

fixes #179

The pull option was set as the default that will cause to download latest version when it is available, which is not desired as discussed in #179. In most case, user want to keep on using the image they pulled in the first place. New version image download is better happened when explicitly triggered by setting pull.

This commit first to make --no-pull the default of --pull/no-pull option. The pull logic tweaks a bit that when the image does not exist, it will anyway trigger the pull. If the image exists, the --pull need to be set to trigger the pull.

@unkcpz unkcpz requested a review from danielhollas July 3, 2023 07:25
@codecov
Copy link

codecov bot commented Jul 3, 2023

Codecov Report

Patch coverage: 42.85% and project coverage change: -0.48 ⚠️

Comparison is base (052f354) 86.76% compared to head (37c58f7) 86.29%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #184      +/-   ##
==========================================
- Coverage   86.76%   86.29%   -0.48%     
==========================================
  Files           9        9              
  Lines         907      912       +5     
==========================================
  Hits          787      787              
- Misses        120      125       +5     
Flag Coverage Δ
py-3.10 86.18% <42.85%> (-0.48%) ⬇️
py-3.11 86.18% <42.85%> (-0.26%) ⬇️
py-3.8 86.13% <42.85%> (-0.26%) ⬇️
py-3.9 86.24% <42.85%> (-0.26%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
aiidalab_launch/__main__.py 79.49% <42.85%> (-0.64%) ⬇️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@danielhollas danielhollas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good change, thanks @unkcpz. Tested locally, works as expected. 🚀
When this is merged I'd suggest to publish a new version of aiidalab-launch, there are a bunch of updates already.

@unkcpz
Copy link
Member Author

unkcpz commented Jul 5, 2023

When this is merged I'd suggest to publish a new version of aiidalab-launch

Yes, agree.

@unkcpz unkcpz merged commit 2a27bdd into main Jul 5, 2023
@unkcpz unkcpz deleted the fix/179/no-pull-when-image-exist branch July 5, 2023 07:25
@unkcpz
Copy link
Member Author

unkcpz commented Jul 10, 2023

@danielhollas v2023.1019 released! 🚀

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.

NEVER use lastest tag
2 participants