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

Change default for /search endpoint with package filter #301

Merged
merged 2 commits into from
Mar 30, 2020

Conversation

ruflin
Copy link
Contributor

@ruflin ruflin commented Mar 25, 2020

So far when a /search?package=foo query was run, by default all versions of the package found were returned. This was in contrast to that by default only the most recent version of a package is returned. To make the API less surprising the behavior was changed that by default only the most recent package is returned.

As there might be still a need to return all packages, and additional param all=true was added. This can now be used in combination with all query params. To get the same result as before, /search?package=foo&all=true has to be run. By default all is false.

@ruflin ruflin requested review from jfsiii and neptunian March 25, 2020 13:53
@ruflin ruflin self-assigned this Mar 25, 2020
@ruflin ruflin force-pushed the search-all-change branch 2 times, most recently from f1f3e94 to 7289fb4 Compare March 25, 2020 20:32
@neptunian
Copy link
Contributor

neptunian commented Mar 26, 2020

@ruflin Can you update the README.md to /search's new behavior.

@jfsiii
Copy link

jfsiii commented Mar 26, 2020

What about a query/filter param which takes a semver range? Perhaps versions which we have elsewhere in the API. So versions=*, versions=7.x, etc

Another option is something more explicit like include or filter (depending on the assumption). I think any of these are preferable to a Boolean param.

@ruflin
Copy link
Contributor Author

ruflin commented Mar 26, 2020

@jfsiii I'm happy to implement some of the suggestion above. I think what I want to first fix in this PR is the surprising behaviour that when just running /search it returns 1 item per package and when adding a package filter, it returns all. I don't think this is intuitive. I would like to keep the default behaviour of only the newest. If we don't need the all at the moment, I can also remove it from the PR and just go with the query change.

So far when a `/search?package=foo` query was run, by default all versions of the package found were returned. This was in contrast to that by default only the most recent version of a package is returned. To make the API less surprising the behavior was changed that by default only the most recent package is returned.

As there might be still a need to return all packages, and additional param `all=true` was added. This can now be used in combination with all query params. To get the same result as before, `/search?package=foo&all=true` has to be run. By default all is false.

add test for all

add one more all test
@ruflin ruflin force-pushed the search-all-change branch from 7289fb4 to 3ff2153 Compare March 30, 2020 19:35
@ruflin ruflin merged commit 0f72e1a into elastic:master Mar 30, 2020
@ruflin ruflin deleted the search-all-change branch March 30, 2020 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants