Skip to content

Implement container prune #904

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.

Sign up for GitHub

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

Jump to bottom
Open
tico88612 wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Implement container prune #904

tico88612 wants to merge 5 commits into from

Conversation

@tico88612
Copy link
Contributor

@tico88612 tico88612 commented Nov 20, 2025 •
edited
Loading

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update

Motivation and Context

Fixed #892

I'm a new contributor here. If there are any better ways to write it, please let me know.

Part of the function references content from PR #902. This PR will be rebased after that PR is merged.

Testing

  • Tested locally
  • Added/updated tests
  • Added/updated docs

@tico88612 tico88612 force-pushed the feat/container-prune branch 2 times, most recently from 104bdd2 to 589061e Compare November 20, 2025 18:21
@tico88612 tico88612 marked this pull request as ready for review November 20, 2025 18:22
@tico88612
Copy link
Contributor Author

tico88612 commented Nov 21, 2025

cc @jglogan @realrajaryan

saehejkang
saehejkang reviewed Nov 21, 2025
View reviewed changes
@saehejkang
Copy link
Contributor

saehejkang commented Nov 21, 2025

This is also missing integration tests. Again, here is a good PR you can use to help make those.

@tico88612 tico88612 force-pushed the feat/container-prune branch from 589061e to 46d4018 Compare November 21, 2025 04:28
@tico88612
Copy link
Contributor Author

tico88612 commented Nov 21, 2025

This is also missing integration tests. Again, here is a good PR you can use to help make those.

Uh, thanks. I just wonder how to write the testcase.

@tico88612 tico88612 force-pushed the feat/container-prune branch 5 times, most recently from 829c989 to 0f959a7 Compare November 21, 2025 15:57
@tico88612
Copy link
Contributor Author

tico88612 commented Nov 21, 2025

Ok, testcases are ready.

@tico88612 tico88612 mentioned this pull request Nov 22, 2025
Open
1 task
@tico88612 tico88612 force-pushed the feat/container-prune branch from 0f959a7 to d6379bf Compare November 22, 2025 02:59
saehejkang
saehejkang reviewed Nov 23, 2025
View reviewed changes
saehejkang
saehejkang reviewed Nov 23, 2025
View reviewed changes
@tico88612 tico88612 force-pushed the feat/container-prune branch from d6379bf to a327ffb Compare November 23, 2025 13:39
@saehejkang saehejkang mentioned this pull request Dec 3, 2025
Merged
7 tasks
@tico88612 tico88612 force-pushed the feat/container-prune branch from a327ffb to b50f515 Compare December 4, 2025 04:31
@saehejkang
Copy link
Contributor

saehejkang commented Dec 9, 2025

After a lengthy discussion with maintainers, on #914, we are going to be focusing on refactoring any prune commands to take a client-side approach. Looks like you took a server-side approach (which is inherently not wrong), but this PR should be updated to reflect the new approach. Take a look at #914 and #940 for a template/reference. Feel free to ping me or the maintainers with any questions.

@tico88612
Implement container prune
a3963e3 
Signed-off-by: ChengHao Yang <17496418+tico88612@users.noreply.github.com>
@tico88612
Add container prune docs
196acd4 
Signed-off-by: ChengHao Yang <17496418+tico88612@users.noreply.github.com>
@tico88612
Test: add getContainerId in CLITest.swift
db3d1f0 
Signed-off-by: ChengHao Yang <17496418+tico88612@users.noreply.github.com>
@tico88612
Add container prune testcases
8acf628 
Signed-off-by: ChengHao Yang <17496418+tico88612@users.noreply.github.com>
@tico88612
Refactor: client side prune container
8a28cd5 
Signed-off-by: ChengHao Yang <17496418+tico88612@users.noreply.github.com>
@tico88612 tico88612 force-pushed the feat/container-prune branch from b50f515 to 8a28cd5 Compare December 13, 2025 16:54
@tico88612
Copy link
Contributor Author

tico88612 commented Dec 16, 2025

Hi @jglogan @realrajaryan, could you take a look on this? I believe this's refactor prune commands to client-side approach.

@jglogan jglogan self-requested a review December 17, 2025 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jglogan jglogan Awaiting requested review from jglogan

1 more reviewer

@saehejkang saehejkang saehejkang left review comments

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Request]: Storage - Add container prune command to remove stopped containers

2 participants

@tico88612 @saehejkang