support nerdctl search #4660
support nerdctl search #4660
Conversation
ChengyuZhu6
force-pushed
the
search
branch
6 times, most recently
from
dd74594 to
cf1a348
Compare
|
Just rebase to main, no code change. |
ChengyuZhu6
force-pushed
the
search
branch
4 times, most recently
from
180e6bf to
8e001e9
Compare
support nerdctl search command Signed-off-by: ChengyuZhu6 <hudson@cyzhu.com>
| flags := cmd.Flags() | ||
|
|
||
| flags.Bool("no-trunc", false, "Don't truncate output") | ||
| flags.StringSlice("filter", nil, "Filter output based on conditions provided") |
There was a problem hiding this comment.
It's a minor point, but It needs to have a short option (-f) as well.
|
|
||
| - :whale: `--limit`: Max number of search results (default: 0) | ||
| - :whale: `--no-trunc`: Don't truncate output (default: false) | ||
| - :whale: `--filter`: Filter output based on conditions provided |
There was a problem hiding this comment.
Regarding the above point, it needs -f.
The filtering flag (-f or --filter) format is a key=value pair. If there is more than one filter, then pass multiple flags (e.g. --filter is-official=true --filter stars=3).
| if !validFilterKeys[key] { | ||
| return nil, fmt.Errorf("invalid filter '%s'", key) | ||
| } |
There was a problem hiding this comment.
I think it would be clearer to handle this processing in the default case of validateFilterValue.
Suggestion:
func validateFilterValue(key, value string) error {
switch key {
case "stars":
if _, err := strconv.Atoi(value); err != nil {
return fmt.Errorf("invalid filter 'stars=%s'", value)
}
case "is-official":
if _, err := strconv.ParseBool(value); err != nil {
return fmt.Errorf("invalid filter 'is-official=%s'", value)
}
default:
return fmt.Errorf("invalid filter '%s'", key)
}
return nil
}| filterMap := make(map[string]string) | ||
| for _, f := range options.Filters { | ||
| parts := strings.SplitN(f, "=", 2) | ||
| if len(parts) != 2 { | ||
| return nil, fmt.Errorf("bad format of filter (expected name=value)") | ||
| } | ||
| key := parts[0] | ||
| value := parts[1] | ||
| if !validFilterKeys[key] { | ||
| return nil, fmt.Errorf("invalid filter '%s'", key) | ||
| } | ||
| if err := validateFilterValue(key, value); err != nil { | ||
| return nil, err | ||
| } | ||
| filterMap[key] = value | ||
| } |
There was a problem hiding this comment.
Thank you for adding error handlings!
But, what do you think about handling the creation of filterMap and error processing in a separate function before making the HTTP request to the registry?
In the current implementation, a client sends an HTTP request to the registry, receives the response, and then checks the values specified in --filter.
If we could determine whether an invalid value was specified in --filter before sending the HTTP request, we could avoid unnecessary HTTP requests.
Suggestion:
Define a function like createFilterMap to check if --filter contains an invalid value and create the filterMap. Call this function before sending the request to the registry to handle errors and create the filterMap.
Details
func createFilterMap(options types.SearchOptions) (map[string]string, error) {
filterMap := make(map[string]string)
for _, f := range options.Filters {
parts := strings.SplitN(f, "=", 2)
if len(parts) != 2 {
return nil, fmt.Errorf("bad format of filter (expected name=value)")
}
key, value := parts[0], parts[1]
if err := validateFilterValue(key, value); err != nil {
return nil, err
}
filterMap[key] = value
}
return filterMap, nil
}| return err | ||
| } | ||
| for _, r := range results { | ||
| if err := tmpl.Execute(stdout, r); err != nil { |
There was a problem hiding this comment.
I apologize for not pointing this out last time, but when specifying --format, the description is displayed in full. Therefore, truncation processing is necessary.
Like in docker:
$ docker search library/ubuntu --filter stars=100 --format json
{"Description":"Ubuntu is a Debian-based Linux operating sys…","IsOfficial":"true","Name":"ubuntu","StarCount":"17750"}
{"Description":"Squid is a caching proxy for the Web. Long-t…","IsOfficial":"false","Name":"ubuntu/squid","StarCount":"122"}
{"Description":"Nginx, a high-performance reverse proxy \u0026 we…","IsOfficial":"false","Name":"ubuntu/nginx","StarCount":"134"}
{"Description":"BIND 9 is a very flexible, full-featured DNS…","IsOfficial":"false","Name":"ubuntu/bind9","StarCount":"117"}
|
Hi Hudson |
3 participants
No description provided.