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

bug 729679 and bug 746609: admin search index browser and unified doctype #582

Closed
wants to merge 5 commits into from
Closed

bug 729679 and bug 746609: admin search index browser and unified doctype #582

wants to merge 5 commits into from

Conversation

willkg
Copy link
Member

@willkg willkg commented Apr 26, 2012

Oh man... This is pretty intense. I tried to touch as many files as I could. The commits are well-contained so if you want to browse it commit-by-commit, that might be easier.

First, this covers bug 729679 which adds the admin search index browser. That was in another pull request, so that's probably fine. Sorry for including it again here, but it would have been tricky to extract.

Second, this covers bug 746609 which reworks all the ES bits so that we use a single doctype for everything and filter on the 'model' field when we want results from a specific bucket. We're still doing bucketed search, but we're now in a good position to write a unified search results view hidden by a waffle flag.

I also cleaned up some code that I accidentally touched and then had to finish cleaning. I added a note to a test that fails periodically. I fixed import statements across the codebase to pull things from wiki.config rather than wiki.models.

r?

This adds an elasticsearch index browser to the admin. This will make it
**way** easier to debug certain classes of problems that require answers
to questions such as:

* "Did xyz get updated in the index?"
* "What the hell did we put in the index and does it bite?"
This is kind of intense. It reworks all the indexing and searching
code so that we're using a single ES doctype in the index. Documents
can be distinguished by their 'model' field which indicates which
model they derive from.

This pulls in a change to elasticutils that adds a get_doctype()
to S. Previously the doctype was based on cls._meta.db_table and that
made a unified doctype difficult.

I added a .search() method to SearchMixin. This is the class method
to use to get an S that is tuned for that model.

I changed a bunch of tests to use the new .search() rather than building
their own S or Sphilastic.

Additionally, this does some minor cleanup of related code.

One thing this doesn't do is rework how the AAQ suggestions work. That's
still a single function that does its thing. Need to keep an eye on that
when we make future changes so we make sure it doesn't break.
'id': {'type': 'integer'},
'id': {'type': 'long'},
'model': {'type': 'string', 'index': 'not_analyzed',
'store': 'yes'},
Copy link
Member Author

Choose a reason for hiding this comment

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

'model' holds the name of the model. Instead of searching by doctype, we're going to filter on the model. The model value is cls._meta.db_table.

This fixes get_doctype_stats so it doesn't use Model.search() which
always looks at the READ_INDEX. We need get_doctype_stats to look
at either the READ_INDEX or the WRITE_INDEX.
@@ -18,7 +18,8 @@
from dashboards import LAST_30_DAYS, PERIODS
from sumo.urlresolvers import reverse
from sumo.redis_utils import redis_client, RedisError
from wiki.models import (Document, MEDIUM_SIGNIFICANCE, MAJOR_SIGNIFICANCE,
from wiki.models import Document
from wiki.config import (MEDIUM_SIGNIFICANCE, MAJOR_SIGNIFICANCE,
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, thanks for cleaning all these up.

@rlr
Copy link
Contributor

rlr commented Apr 26, 2012

All looks good and tests pass. Reindexing is almost done so I am going to go ahead and r+

@willkg
Copy link
Member Author

willkg commented Apr 27, 2012

Landed in 982def1 and fdafc92 in master.

@willkg willkg closed this Apr 27, 2012
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.

None yet

2 participants