Skip to content
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

cert: do not limit internal searches in cert-find #634

Closed
wants to merge 1 commit into from
Closed

cert: do not limit internal searches in cert-find #634

wants to merge 1 commit into from

Conversation

HonzaCholasta
Copy link
Contributor

@HonzaCholasta HonzaCholasta commented Mar 22, 2017

Instead, apply the limits on the combined result.

This fixes (absence of) --sizelimit leading to strange behavior, such as
cert-find --users user returning a non-empty result only with
--sizelimit 0.

https://pagure.io/freeipa/issue/6716

@stlaz stlaz self-requested a review March 22, 2017 08:38
@stlaz stlaz self-assigned this Mar 22, 2017
@stlaz
Copy link
Contributor

stlaz commented Mar 22, 2017

The tests obviously fail as they expect the cert-find command to respect the sizelimit option.

@HonzaCholasta
Copy link
Contributor Author

The fix was incomplete, it should be OK now.

Instead, apply the limits on the combined result.

This fixes (absence of) `--sizelimit` leading to strange behavior, such as
`cert-find --users user` returning a non-empty result only with
`--sizelimit 0`.

https://pagure.io/freeipa/issue/6716
@stlaz
Copy link
Contributor

stlaz commented Mar 23, 2017

Works for me.

@stlaz stlaz added the ack Pull Request approved, can be merged label Mar 23, 2017
@martbab
Copy link
Contributor

martbab commented Mar 27, 2017

master:

  • 6de507c cert: do not limit internal searches in cert-find
    ipa-4-5:

  • 6382f9e cert: do not limit internal searches in cert-find

@martbab martbab added the pushed Pull Request has already been pushed label Mar 27, 2017
@martbab martbab closed this Mar 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ack Pull Request approved, can be merged pushed Pull Request has already been pushed
Projects
None yet
3 participants