how to reuse only the "get images" logic of image.List

[davidhsingyuchen]

What is the problem you're trying to solve

Currently image.List contains 2 steps:

1. Get the images according to the filters.
2. Print the images.

It'd be nice to have an exported function that only does the first step. The detailed rationale is very similar to #1837 (comment), and this time instead of directly creating a PR, I think opening an issue first probably makes more sense.

Describe the solution you'd like

Looking at the fields of types.ImageListCommandOptions, many of them are only needed by 2. Print the images, so maybe we can create a new function and make it only depend on the things it needs:

func ListTheNewFunc(ctx context.Context, namespace, address string, Filters, NameAndRefFilter []string) ([]images.Image, error)

or maybe it can just take a *containerd.Client as the caller of this function can be potentially a long-running process, and it can call clientutil.NewClient itself to instantiate the client only once during process initialization:

func ListTheNewFunc(ctx context.Context, client *containerd.Client, Filters, NameAndRefFilter []string) ([]images.Image, error)

I don't have a perfect name in mind for the new function, so I just use ListTheNewFunc as a placeholder, and I'm not even sure if creating a new function makes sense to you, and I appreciate any suggestion, thanks!

Additional context

I'm more than happy to work on this as long as the community reaches a consensus on the approach.

[djdongjin]

Maybe you can rename the current image.List to image.ListCommandHandler, and extract the main list logic to a new image.List func. You can pass a client to the new image.List (e.g., as your 2nd func). Something like this:

// ListCommandHandler constructs containerd client, calls `List` to get images that match filters, and formats/prints the result.
func ListCommandHandler(ctx context.Context, options types.ImageListCommandOptions) error {
    ...
    imageList, err :=List(ctx, client, ...)
    ...
}

// List queries containerd client to get image list and only returns those matching given filters.
func List(ctx context.Context, client *containerd.Client, Filters, NameAndRefFilter []string) ([]images.Image, error)

---

another thing we can do for this command is to pass a io.Writer to printImages, which can come from ListCommandHandler, which can come from cmd.OutOrStdout when ListCommandHandler is called by imagesAction.

Just notice currently the output is binded to os.Stdout.

nerdctl/pkg/cmd/image/list.go

Lines 186 to 187 in cfe96a9

	var w io.Writer
	w = os.Stdout

[davidhsingyuchen]

Maybe you can rename the current image.List to image.ListCommandHandler, and extract the main list logic to a new image.List func. You can pass a client to the new image.List (e.g., as your 2nd func).

Sounds great! @AkihiroSuda @Zheaoli Is it possible for us to consistently use this naming pattern when we extract the "main logic" out of a command? Thanks!

// ListCommandHandler constructs containerd client, calls List to get images that match filters, and formats/prints the result.
func List(ctx context.Context, options types.ImageListCommandOptions) error {

typo: List -> ListCommandHandler?

another thing we can do for this command is to pass a io.Writer to printImages, which can come from ListCommandHandler, which can come from cmd.OutOrStdout when ListCommandHandler is called by imagesAction.

Just notice currently the output is binded to os.Stdout.

Sure, I can do it in another PR. Just want to make it clear that this may be unrelated to this issue. Linking the relevant issue: #1856

[djdongjin]

typo: List -> ListCommandHandler?

yes 😅

Sure, I can do it in another PR. Just want to make it clear that this may be unrelated to this issue. Linking the relevant issue: #1856

SGTM!

[davidhsingyuchen]

@AkihiroSuda @Zheaoli Would greatly appreciate your feedback for #1855 (comment), thank you!

"main logic" is from #1837 (comment):

1. preprocess step: process cli flag and construct options (which we're refactoring);
2. flag/option validation step: check either flags (previously) or options (after refactor), to resolve conflict options or do some filtering (e.g., here --quiet and --filter=size; in run we have some -i -d -t conflict).
3. main logic step: which I feel we should return the structs if it's a GET/LIST-kind func, or a success or fail indicator (bool, err, etc), or a PUT/DELETE-kind func.
4. postprocess step: after receiving results from 3, print as a table or json, etc.

From #1837 (comment):

LsCmd or other names are not keeping the same style as other command

Maybe we can make the names consistent regardless if the "main logic" has been separated out? To be specific, we can rename all functions to be XXXCommandHandler first and then we can separate the "main logic" out of each function in follow-up PRs.