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.

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

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

Merged
merged 1 commit into from Jun 3, 2016

Conversation

Projects
None yet
5 participants
@yongtang
Member

yongtang commented May 30, 2016

- 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

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah May 31, 2016

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

Member

thaJeztah commented May 31, 2016

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

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah May 31, 2016

Member

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

Member

thaJeztah commented May 31, 2016

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

@yongtang

This comment has been minimized.

Show comment
Hide comment
@yongtang

yongtang Jun 1, 2016

Member

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.

Member

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

This comment has been minimized.

Show comment
Hide comment
@vdemeester

vdemeester Jun 1, 2016

Member

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

Member

vdemeester commented Jun 1, 2016

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

@yongtang

This comment has been minimized.

Show comment
Hide comment
@yongtang

yongtang Jun 1, 2016

Member

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

Member

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

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Jun 1, 2016

Member

@yongtang It's merged 👍

Member

thaJeztah commented Jun 1, 2016

@yongtang It's merged 👍

@yongtang

This comment has been minimized.

Show comment
Hide comment
@yongtang

yongtang Jun 1, 2016

Member

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 )

Member

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

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Jun 1, 2016

Member

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

Member

thaJeztah commented Jun 1, 2016

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

yongtang added a commit to yongtang/docker that referenced this pull request Jun 1, 2016

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>
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>
@yongtang

This comment has been minimized.

Show comment
Hide comment
@yongtang

yongtang Jun 3, 2016

Member

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.

Member

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

This comment has been minimized.

Show comment
Hide comment
@vdemeester

vdemeester Jun 3, 2016

Member

LGTM 🐮

Member

vdemeester commented Jun 3, 2016

LGTM 🐮

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Jun 3, 2016

Member

LGTM, thanks!

Member

thaJeztah commented Jun 3, 2016

LGTM, thanks!

@vdemeester

This comment has been minimized.

Show comment
Hide comment
@vdemeester

vdemeester Jun 3, 2016

Member

docs LGTM 👼

Member

vdemeester commented Jun 3, 2016

docs LGTM 👼

@thaJeztah thaJeztah merged commit 020a86b into moby: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

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Jun 3, 2016

Member

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

Member

thaJeztah commented Jun 3, 2016

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

@yongtang yongtang deleted the yongtang:23055-docker-search-limit branch Jun 3, 2016

@jimmyxian jimmyxian referenced this pull request Jun 6, 2016

Open

Keep track of the missing API in Swarm #2333

4 of 23 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment