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

[Fix #4407] Port Project Search for Elasticsearch 6.x #4408

Merged

Conversation

safwanrahman
Copy link
Member

@safwanrahman safwanrahman commented Jul 19, 2018

This was missed from our intial stage while we port our search infra to Elasticsearch 6.x
Its similar to the File Search that we ported in #4211
Its needed to be merged before we deploy new search to production.
This fixes #4407. @ericholscher r?

@safwanrahman safwanrahman self-assigned this Jul 19, 2018
@safwanrahman safwanrahman added the Priority: high High priority label Jul 19, 2018
@agjohnson agjohnson removed the Priority: high High priority label Jul 20, 2018
@agjohnson agjohnson added this to the 2.7 milestone Jul 20, 2018
Copy link
Member

@ericholscher ericholscher left a comment

Choose a reason for hiding this comment

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

This looks like a good update. I do wonder if this is useful, or if we should just be dropping users into the main site search with the project filtered. It kinda feels weird to have this search UI which doesn’t support Faceting and filtering. I guess it matches what users get on the doc page search, but you can’t even specify a version to search. I’m thinking maybe we just remove it, unless there is a good reason to keep it?

@safwanrahman
Copy link
Member Author

I think its a design decision to remove it. But as we need to deploy our ES changes soon, we should keep it as it is and port it to work with latest code. Later we can discuss about removing it.

What do you think?

@ericholscher
Copy link
Member

Sounds good. 👍

@ericholscher ericholscher merged commit 665cc08 into readthedocs:search_upgrade Jul 24, 2018
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