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

fix: localnet displays a warning when image is out of date #308

Merged
merged 12 commits into from
Aug 21, 2023

Conversation

inaie-makerx
Copy link
Contributor

@github-actions
Copy link

github-actions bot commented Aug 8, 2023

Coverage

Coverage Report
FileStmtsMissCoverMissing
src/algokit
   __init__.py15753%6–13, 17–24, 32–34
   __main__.py220%1–3
src/algokit/cli
   completions.py105298%80, 95
   deploy.py56591%34–36, 78, 96
   doctor.py48394%142–144
   generate.py57198%116
   goal.py30197%42
   init.py1901692%268–269, 319, 322–324, 335, 379, 405, 445, 454–456, 459–464, 477
   localnet.py93397%162, 183–184
src/algokit/core
   bootstrap.py1612485%103–104, 126, 149, 214, 217, 223–237, 246–251
   conf.py54885%10, 24, 28, 36, 38, 71–73
   deploy.py691184%61–64, 73–75, 79, 84, 91–93
   doctor.py65789%67–69, 92–94, 134
   generate.py41295%68, 86
   log_handlers.py68790%50–51, 63, 112–116, 125
   proc.py45198%98
   sandbox.py1811592%100–107, 118, 277, 293, 308–310, 326
   typed_client_generation.py80594%55–57, 70, 75
   version_prompt.py73889%27–28, 40, 59–62, 80, 109
TOTAL159712892% 

Tests Skipped Failures Errors Time
242 0 💤 0 ❌ 0 🔥 20.486s ⏱️

tests/localnet/conftest.py Show resolved Hide resolved
tests/localnet/conftest.py Outdated Show resolved Hide resolved
src/algokit/core/sandbox.py Outdated Show resolved Hide resolved
src/algokit/core/sandbox.py Outdated Show resolved Hide resolved
src/algokit/core/sandbox.py Outdated Show resolved Hide resolved
tests/localnet/test_localnet_start.py Show resolved Hide resolved
@aorumbayev aorumbayev self-requested a review August 16, 2023 07:28
Copy link
Collaborator

@aorumbayev aorumbayev left a comment

Choose a reason for hiding this comment

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

@inaie-makerx unresolved one of the comments as the bug still persists, please address and I'll approve if all good.

@aorumbayev aorumbayev self-requested a review August 16, 2023 13:42
Copy link
Collaborator

@aorumbayev aorumbayev left a comment

Choose a reason for hiding this comment

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

@inaie-makerx approving but please address the last comment with regards to values you are mocking for inspect command. Before merging, please make sure that conventional commit prefix is set to fix instead of a feat as this is more of a minor enhancement into existing localnet features so a patch bump is more appropriate than a minor bump.

@daniel-makerx any chance you can take one extra look as well so we get second approval for this PR?

arg = '{{index (split (index .RepoDigests 0) "@") 1}}'
proc_mock.set_output(
["docker", "image", "inspect", ALGORAND_IMAGE, "--format", arg],
["[algorand/algod@sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb\n]"],
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should be just sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb\n without square brackets due to {{index (split (index .RepoDigests 0) "@") 1}}

Copy link
Collaborator

Choose a reason for hiding this comment

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

These are still in place? Am i missing something? I think you pushed a commit to remove them from the test but unsure if these are still correct, the output should not contain [{title}/{image title}@sha256:{hash}] = we parse it out via go templating in format param.


proc_mock.set_output(
["docker", "image", "inspect", INDEXER_IMAGE, "--format", arg],
["[makerxau/algorand-indexer-dev@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n]"],
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here, should be just sha256:aa....\n

Copy link
Collaborator

Choose a reason for hiding this comment

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

These are still in place? Am i missing something? I think you pushed a commit to remove them from the test but unsure if these are still correct, the output should not contain [{title}/{image title}@sha256:{hash}] = we parse it out via go templating in format param.

@aorumbayev aorumbayev changed the title feat: localnet warning when image is out date fix: localnet displays a warning when image is out of date Aug 16, 2023
@aorumbayev aorumbayev requested a review from robdmoore August 17, 2023 07:56
@robdmoore
Copy link
Contributor

Nice!

@aorumbayev aorumbayev merged commit be5a5df into main Aug 21, 2023
@aorumbayev aorumbayev deleted the localnet-warning-update branch August 21, 2023 08:09
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.

5 participants