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

RFC merge RECORDS_REST_SORT_OPTIONS inside RECORDS_REST_ENDPOINTS #104

Closed
hachreak opened this issue Jul 19, 2016 · 3 comments
Closed

RFC merge RECORDS_REST_SORT_OPTIONS inside RECORDS_REST_ENDPOINTS #104

hachreak opened this issue Jul 19, 2016 · 3 comments

Comments

@hachreak
Copy link
Member

hachreak commented Jul 19, 2016

Problem

Currently we have a special configuration called RECORDS_REST_SORT_OPTIONS that specifies the sort configuration for each index.

  1. It can be error prone because the user have to specify the index name inside RECORDS_REST_ENDPOINTS and, in the same time, inside RECORDS_REST_SORT_OPTIONS.
  2. In addition, It is not intuitive because the convention for the dictionary key is different from RECORDS_REST_ENDPOINTS (where as key is used the pid_type).
  3. Inside sorter.py we set the sort options to the search_class. So, why we don't do it directly?

Proposal
Can we directly specify the default sort configuration inside the search class and remove RECORDS_REST_SORT_OPTIONS?


cc @inveniosoftware/triagers
see #99

@lnielsen
Copy link
Member

RECORDS_REST_SORT_OPTIONS is also used by UI: https://github.com/zenodo/zenodo/blob/master/zenodo/config.py#L508

I think this needs to be treated as part of how Records-REST should be configured - see #64

The reason to keep it separate is because the sorting is fully independent of the search. You can rip it out and replace it with something else.

@hachreak
Copy link
Member Author

could you share the configuration inside the seach_class as you say in #64 proposal 1?
It looks like the natural point where share the configuration for facets, sort, index, doc_type and max_result_window.

@jirikuncar
Copy link
Member

I'm also for keeping it separate as it's not part of the main search API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants