Add `--limit` option to `docker search` #23107

Merged
merged 1 commit into from Jun 3, 2016

Projects

None yet

5 participants

@yongtang
Contributor

- What I did

This fix tries to address the issue raised in #23055. Currently docker search result caps at 25 and there is no way to allow getting more results (if exist).

This fix adds the flag --limit so that it is possible to return more results from the docker search.

- How I did it

Additional flag --limit has been added to docker search. Related documentation has been updated.

- How to verify it

Additional tests have been added to cover the changes.

- Description for the changelog

Add --limit option to docker search so that docker search could return different number of search results.

- A picture of a cute animal (not mandatory but encouraged)

This fix fixes #23055.

NOTE: Pull request for engine-api will be created separately.

Signed-off-by: Yong Tang yong.tang.github@outlook.com

@thaJeztah
Member

I tested this, and it works, but we may need to add some validation / boundary checks;

Values <= 0 are treated as "1" it seems;

docker search --limit=0 a
NAME      DESCRIPTION                                     STARS     OFFICIAL   AUTOMATED
ubuntu    Ubuntu is a Debian-based Linux operating s...   4008      [OK]

docker search --limit=-1 a
NAME      DESCRIPTION                                     STARS     OFFICIAL   AUTOMATED
ubuntu    Ubuntu is a Debian-based Linux operating s...   4008      [OK]

The maximum limit (I suspect, as set by Docker Hub), is 100, any value over that still returns 100 results.

I think we should at least document that, perhaps add an error message for values < 1 or > 100

@thaJeztah
Member

Design looks good to me though, so I'm moving to code review

@yongtang
Contributor
yongtang commented Jun 1, 2016

Thanks @thaJeztah. The pull request has been updated with value check between [1, 100]. The documentation has been updated as well. Please let me know if there are any issues.

@vdemeester
Member

@yongtang you can make a PR to engine-api too now (design-review passed 👍)

@yongtang yongtang referenced this pull request in docker/engine-api Jun 1, 2016
Merged

Add `--limit` option to `docker search` #250

@yongtang
Contributor
yongtang commented Jun 1, 2016

Thanks @vdemeester!
I created a pull request in engine-api:
docker/engine-api#250
Let me know if there are any issues.

@thaJeztah
Member

@yongtang It's merged 👍

@yongtang
Contributor
yongtang commented Jun 1, 2016

Thanks for the help @thaJeztah @vdemeester!
It seems that both this pull request (#23107) and another pull request (#23090) have updated engine-api and there are some overlapping conflict.
I am wondering if it is possible to review #23090 first? After #23090 is merged it will be much easier to update this pull request ( #23107 )

@thaJeztah
Member

@yongtang just reviewed that PR; had one minor nit, but looks ok to go after that

@yongtang yongtang added a commit to yongtang/docker that referenced this pull request Jun 1, 2016
@yongtang yongtang Vendor engine-api to 56ad47e075bd7a47a33412e56cc298d56ef2f514
This fix updates engine-api to 56ad47e075bd7a47a33412e56cc298d56ef2f514.

This fix is related to #23055 and #23107.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
cfe0416
@yongtang yongtang Add `--limit` option to `docker search`
This fix tries to address the issue raised in #23055.
Currently `docker search` result caps at 25 and there is
no way to allow getting more results (if exist).

This fix adds the flag `--limit` so that it is possible
to return more results from the `docker search`.

Related documentation has been updated.

Additional tests have been added to cover the changes.

This fix fixes #23055.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
92f10fe
@yongtang
Contributor
yongtang commented Jun 3, 2016

Thanks @thaJeztah @vdemeester for the review.

The pull request has been rebase to reflect the changes in engine-api (PR #23208). Please let me know if there are any issues.

@vdemeester
Member

LGTM 🐮

@thaJeztah
Member

LGTM, thanks!

@vdemeester
Member

docs LGTM 👼

@thaJeztah thaJeztah merged commit 020a86b into docker:master Jun 3, 2016

8 checks passed

docker/dco-signed All commits signed
Details
documentation success
Details
experimental Jenkins build Docker-PRs-experimental 19360 has succeeded
Details
gccgo Jenkins build Docker-PRs-gccgo 6222 has succeeded
Details
janky Jenkins build Docker-PRs 28166 has succeeded
Details
userns Jenkins build Docker-PRs-userns 10330 has succeeded
Details
win2lin Jenkins build Docker-PRs-Win2Lin 26695 has succeeded
Details
windowsTP5 Jenkins build Docker-PRs-WoW-TP5 2508 has succeeded
Details
@thaJeztah thaJeztah added this to the 1.12.0 milestone Jun 3, 2016
@thaJeztah
Member

oh, and ping @albers, @sdurrheimer (for the new flag)

@yongtang yongtang deleted the yongtang:23055-docker-search-limit branch Jun 3, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment