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

feat(BA-997): Add image purge CLI command for hard deleting ImageRow #3951

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

jopemachine
Copy link
Member

@jopemachine jopemachine commented Mar 12, 2025

Resolves #3979 (BA-997).

Checklist: (if applicable)

  • Mention to the original issue

📚 Documentation preview 📚: https://sorna--3951.org.readthedocs.build/en/3951/


📚 Documentation preview 📚: https://sorna-ko--3951.org.readthedocs.build/ko/3951/

@github-actions github-actions bot added size:M 30~100 LoC comp:client Related to Client component comp:cli Related to CLI component labels Mar 12, 2025
@jopemachine jopemachine changed the title fix: Remove wrong Image forget CLI command fix(BA-970): Remove wrong Image forget CLI command Mar 12, 2025
@jopemachine jopemachine added this to the 25Q1 milestone Mar 12, 2025
@jopemachine jopemachine changed the title fix(BA-970): Remove wrong Image forget CLI command fix(BA-970): Wrong Image forget CLI command Mar 12, 2025
@jopemachine jopemachine changed the title fix(BA-970): Wrong Image forget CLI command fix(BA-997): Wrong Image forget CLI command Mar 12, 2025
@jopemachine jopemachine changed the title fix(BA-997): Wrong Image forget CLI command feat(BA-997): Add image purge CLI command for hard deleting ImageRow Mar 12, 2025
@github-actions github-actions bot added size:L 100~500 LoC comp:manager Related to Manager component comp:agent Related to Agent component comp:common Related to Common component require:db-migration Automatically set when alembic migrations are added or updated and removed size:M 30~100 LoC labels Mar 12, 2025
@jopemachine jopemachine force-pushed the fix/wrong-forget-cli-impl branch from d1fdd5c to 8c373c2 Compare March 12, 2025 08:45
@github-actions github-actions bot added size:M 30~100 LoC and removed size:L 100~500 LoC labels Mar 12, 2025
@jopemachine jopemachine added comp:client Related to Client component comp:cli Related to CLI component and removed comp:agent Related to Agent component comp:client Related to Client component comp:common Related to Common component comp:cli Related to CLI component require:db-migration Automatically set when alembic migrations are added or updated comp:manager Related to Manager component labels Mar 12, 2025
@jopemachine jopemachine marked this pull request as ready for review March 13, 2025 02:04
Comment on lines 122 to 127
if remove_from_registry:
result = session.Image.untag_image_from_registry(image_id)
if not result["ok"]:
print_fail("Image untag has failed: {0}".format(result["msg"]))
sys.exit(ExitCode.FAILURE)
session.Image.purge_image_by_id(image_id)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess it should be managed as a transaction, but is it okay if only one of the actions (remove_from_registry, purge_image_by_id) executes and the other fails?

Copy link
Member Author

Choose a reason for hiding this comment

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

If untag_image_from_registry succeeds but purge_image_by_id fails, an inappropriate ImageRow will remain in the images table.

Unless the purge_image mutation is explicitly called for the corresponding ImageRow, it will continue to exist.

Ideally, a soft deletion process should be added during the image rescan to handle images that have already been removed from the registry.

See BA-969 for this

@jopemachine jopemachine force-pushed the fix/wrong-forget-cli-impl branch from 717e305 to 4ed0f77 Compare March 14, 2025 07:52
@github-actions github-actions bot added size:L 100~500 LoC area:docs Documentations comp:manager Related to Manager component and removed size:M 30~100 LoC labels Mar 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:docs Documentations comp:cli Related to CLI component comp:client Related to Client component comp:manager Related to Manager component size:L 100~500 LoC
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add image purge CLI command for hard deleting ImageRow from DB
2 participants