Adding capability to filter by name, id or status to list containers api #8543
Conversation
| @@ -61,3 +62,22 @@ func FromParam(p string) (Args, error) { | |||
| } | |||
| return args, nil | |||
| } | |||
|
|
|||
| func MatchFilter(source string, filter Args, field string) bool { | |||
There was a problem hiding this comment.
I like idea, but I would like if we can call this like:
psFilters[field].Match(source)
or
psFilters.Match(field, source)
There was a problem hiding this comment.
I used the second option.
77cf7cc to
a80ad0e
Compare
|
Please review, I hope I addressed above mentioned issues. |
| @@ -61,3 +62,22 @@ func FromParam(p string) (Args, error) { | |||
| } | |||
| return args, nil | |||
| } | |||
|
|
|||
| func (filters Args) Match(source string, field string) bool { | |||
There was a problem hiding this comment.
I prefer field be first argument.
a80ad0e to
2ad0151
Compare
|
@LK4D4 Thanks for reviewing, I made changes based on your comments. Technically I agree one test should do only one thing, second half was just trying to demonstrate that regular expression works instead of recoding the whole test again, I did remove that part now. |
| // start exited container | ||
| runCmd := exec.Command(dockerBinary, "run", "-d", "--name=a_name_to_match", "busybox") | ||
| out, _, err := runCommandWithOutput(runCmd) | ||
| errorOut(err, t, out) |
There was a problem hiding this comment.
hey sorry so we got rid of this errorOut function in a cleanup PR, can you rebase and change these to t.Fatalf()
2ad0151 to
75afe2b
Compare
|
Changed the code to replace errorOut with t.Fatal. |
|
one nit, other than that LGTM |
75afe2b to
9e7dd9b
Compare
|
Yep fixed it, my eyes did not pay attention to copy paste code especially comments. Some of the other comments are also weird, why do we say exited container? I am tempted to get that out too. |
|
@brahmaroutu Yup, recheck logic of comments and your tests pls. |
9e7dd9b to
531a25b
Compare
|
I did change the comments. I believe logic can stay as is, it still does filter with both stopped and running containers in the list. |
| } | ||
| firstID := stripTrailingCharacters(out) | ||
|
|
||
| // make sure the exited container is not running |
There was a problem hiding this comment.
you can take this part out, just so it takes out a bit of the time it takes to run the tests
|
I want to have two id's so that I can see if I got the one I wanted. |
|
ya ya for sure we just don't need to run wait, because technically we don't need that container to have exited |
|
that is true, wait is redundant. Thanks. I have remove that code. |
531a25b to
2345242
Compare
|
LGTM |
Closes moby#7599 Signed-off-by: Srini Brahmaroutu <srbrahma@us.ibm.com>
2345242 to
1634625
Compare
|
LGTM |
Adding capability to filter by name, id or status to list containers api
Added more examples and functionalities for docker ps documentation Signed-off-by: André Martins <martins@noironetworks.com>
…s-filter Added missing documentation for PR #8543
Addresses #7599
Signed-off-by: Srini Brahmaroutu srbrahma@us.ibm.com