Skip to content

Conversation

@max-zilla
Copy link
Contributor

Adds "sort" and "order" parameters for search api, with swagger entries. Support for date and numeric fields. I have implementation of "name" with a _sort field but not mentioned in documentation because other strings not yet supported - wanted to include this code as a working example.

Also included is a new /api/deleteindex endpoint for easier reindexing on sharded instances.

Copy link
Member

@bodom0015 bodom0015 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation issue breaks the Swagger API browser. The lack of string support made things a bit confusing, so we may want to return/handle a different error code here (maybe 405 - Method Not Allowed).

In general, the date sorting seemed to work (although, in my case I didn't have any numeric values to sort with)

This may be out-of-scope for this PR, but I also noticed that the /search endpoint doesn't seem to handle errors very well. The Swagger API was returning a strange error that I had never seen before when trying to search on "name" (which you had indicated wouldn't work):

Undocumented | Failed to fetch.
Possible Reasons:
* CORS
* Network Failure
* URL scheme must be "http" or "https" for CORS request

Notably, this is exactly the same error that is produced get if Swagger cannot communicate with Clowder.

An exception was thrown in IntelliJ in this case as well:

play.api.Application$$anon$1: Execution exception[[SearchPhaseExecutionException: all shards failed]]
	at play.api.Application$class.handleError(Application.scala:293) ~[play_2.10-2.2.6.jar:2.2.6]
	at play.api.DefaultApplication.handleError(Application.scala:399) [play_2.10-2.2.6.jar:2.2.6]
	at play.core.server.netty.PlayDefaultUpstreamHandler$$anonfun$13$$anonfun$apply$1.applyOrElse(PlayDefaultUpstreamHandler.scala:170) [play_2.10-2.2.6.jar:2.2.6]
	at play.core.server.netty.PlayDefaultUpstreamHandler$$anonfun$13$$anonfun$apply$1.applyOrElse(PlayDefaultUpstreamHandler.scala:167) [play_2.10-2.2.6.jar:2.2.6]
	at scala.runtime.AbstractPartialFunction.apply(AbstractPartialFunction.scala:33) [scala-library.jar:na]
	at scala.util.Failure$$anonfun$recover$1.apply(Try.scala:185) [scala-library.jar:na]
org.elasticsearch.action.search.SearchPhaseExecutionException: all shards failed
	at org.elasticsearch.action.search.AbstractSearchAsyncAction.onFirstPhaseResult(AbstractSearchAsyncAction.java:206) ~[elasticsearch-2.3.5.jar:2.3.5]
	at org.elasticsearch.action.search.AbstractSearchAsyncAction$1.onFailure(AbstractSearchAsyncAction.java:152) ~[elasticsearch-2.3.5.jar:2.3.5]
	at org.elasticsearch.action.ActionListenerResponseHandler.handleException(ActionListenerResponseHandler.java:46) ~[elasticsearch-2.3.5.jar:2.3.5]
	at org.elasticsearch.transport.TransportService$DirectResponseChannel.processException(TransportService.java:874) ~[elasticsearch-2.3.5.jar:2.3.5]
	at org.elasticsearch.transport.TransportService$DirectResponseChannel.sendResponse(TransportService.java:852) ~[elasticsearch-2.3.5.jar:2.3.5]
	at org.elasticsearch.transport.TransportService$4.onFailure(TransportService.java:389) ~[elasticsearch-2.3.5.jar:2.3.5]
Caused by: org.elasticsearch.search.SearchParseException: No mapping found for [name._sort] in order to sort on
	at org.elasticsearch.search.sort.SortParseElement.addSortField(SortParseElement.java:213) ~[elasticsearch-2.3.5.jar:2.3.5]
	at org.elasticsearch.search.sort.SortParseElement.addCompoundSortField(SortParseElement.java:187) ~[elasticsearch-2.3.5.jar:2.3.5]
	at org.elasticsearch.search.sort.SortParseElement.parse(SortParseElement.java:85) ~[elasticsearch-2.3.5.jar:2.3.5]
	at org.elasticsearch.search.SearchService.parseSource(SearchService.java:856) ~[elasticsearch-2.3.5.jar:2.3.5]
	at org.elasticsearch.search.SearchService.createContext(SearchService.java:667) ~[elasticsearch-2.3.5.jar:2.3.5]
	at org.elasticsearch.search.SearchService.createAndPutContext(SearchService.java:633) ~[elasticsearch-2.3.5.jar:2.3.5]

I wonder if we could handle this SearchParseException more gracefully and return an HTTP error code instead?

Co-authored-by: Mike Lambert <lambert8@illinois.edu>
@max-zilla
Copy link
Contributor Author

max-zilla commented Mar 23, 2021

Thanks @bodom0015 , merged your changes and looking into error. I am getting error looking at Swagger as well:

can not read a block mapping entry; a multiline key may not be an implicit key```
however, this PR doesn't change that line and i get same error on develop branch. investigating.

edit: OK, this error was due to having HTML in my Clowder info customization, breaks the Swagger info block.

@max-zilla
Copy link
Contributor Author

Also updated PR to omit the name._sort autofill until it's properly supported, that way databases that haven't been reindexed don't throw errors.

@max-zilla max-zilla requested review from a team and bodom0015 and removed request for bodom0015, lmarini and robkooper March 23, 2021 20:13
Copy link
Member

@bodom0015 bodom0015 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍 Thank you, @max-zilla!

@lmarini lmarini merged commit 7c58c9d into develop Mar 24, 2021
@robkooper robkooper deleted the allow-sorting-by-metadata-fields branch September 10, 2022 03:28
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.

4 participants