test_reindexed_no_client_charms deletes all indexed data

Bug #1317567 reported by Reed O'Brien
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Fix Released

Bug Description

charmworld/tests/test_search.py: TestReindex.test_reindexed_no_client_charms

Deletes all ES index data requiring re-ingesting charms. The test is commented out in 513, but should be re-written sanely by someone who understands what it should be doing.

Related branches

Revision history for this message
Aaron Bentley (abentley) wrote :

The tests are meant to be run under test.ini, against dummy data stores. They are supposed to have free rein to delete and create anything, and no test should require an ingest to have happened.

It looks like the bug here is that there is no dummy assinged for ElasticSearch.

Revision history for this message
Reed O'Brien (reedobrien) wrote :

I think the issue is that when developing locally -- you run the tests and it destroys any data in your ES dev instance. Which means the developer then needs to re-ingest charms and bundles. The test suite doesn't need them itself. It creates a "temp_index" index to use, but then deletes the developers index in this one test. This has affected other CW devs, too. I was asked to comment it out when I isolated it as the culprit deleting developers working indexes. I don't understand what it is supposed to be testing.

perhaps it needs an @unittest.skipUnless(run_under_testini) or similar decoration.

Revision history for this message
Aaron Bentley (abentley) wrote :
Download full text (3.5 KiB)

Even in the case where the index is deleted, you should be able to restore it with bin/es-update, which is *much* faster than re-ingesting.

The purpose of the test is to ensure reindexing doesn't raise an exception when charms are not supplied and there's no existing index. Here's how to find that kind of thing out in the future:

$ bzr log --show-diff -r annotate:charmworld/tests/test_search.py:1224
revno: 241.1.5
committer: Aaron Bentley <email address hidden>
branch nick: remove-api-0
timestamp: Tue 2013-06-04 11:28:33 -0400
  Handle reindexing when the index does not exist.
=== modified file 'charmworld/search.py'
--- charmworld/search.py 2013-06-04 14:40:57 +0000
+++ charmworld/search.py 2013-06-04 15:28:33 +0000
@@ -233,7 +233,8 @@
         kwargs = {'index': self.index_name}
         if limit is not None:
             kwargs['size'] = limit
- hits = self._client.search(dsl, **kwargs)
+ with translate_error():
+ hits = self._client.search(dsl, **kwargs)
         if limit is None and len(hits['hits']['hits']) < hits['hits']['total']:
             kwargs['size'] = hits['hits']['total']
             hits = self._client.search(dsl, **kwargs)
@@ -365,7 +366,10 @@
             if charms is None:
- charms = self.api_search(valid_charms_only=False)
+ try:
+ charms = self.api_search(valid_charms_only=False)
+ except IndexMissing:
+ charms = []
             return copy
@@ -387,7 +391,10 @@
         aliased = index_client.get_aliased()
         if aliased == []:
- index_client.delete_index()
+ try:
+ index_client.delete_index()
+ except IndexMissing:
+ pass
         index_client.update_aliased(new_index.index_name, aliased)

=== modified file 'charmworld/tests/test_search.py'
--- charmworld/tests/test_search.py 2013-06-04 14:40:57 +0000
+++ charmworld/tests/test_search.py 2013-06-04 15:28:33 +0000
@@ -461,6 +461,11 @@
         self.assertIn('series', copy.get_mapping()['charm']['properties'])
         self.assertEqual({'_id': 'a', 'name': 'bar'}, copy.get('a'))

+ def test_create_replacement_misssing(self):
+ client = ElasticSearchClient.from_settings(get_ini(), 'temp-index')
+ copy = client.create_replacement()
+ self.addCleanup(copy.delete_index)
     def test_put_mapping_incompatible_mapping_error(self):
         # The error we get from an incompatible mapping is IncompatibleMapping
         context = temp_index_client(name='foo', put_mapping=False)
@@ -610,3 +615,8 @@
         mapping = alias.get_mapping()
+ def test_reindexed_no_client_charms(self):
+ client = ElasticSearchClient.from_settings(get_ini())
+ new_client = reindex(client, charms=[])
+ new_client...


Revision history for this message
Reed O'Brien (reedobrien) wrote :

Thanks for that comment. It is helpful for a bzr newb.

Here is the reason I was asked to comment it out.

I have 113 ingested charms:

robrien@erwin:~/py/charmworld/ngrams$ curl localhost:9200/charms/charm/_count?q=* && echo

I develop then I run the tests to ensure nothing is broken (just the culprit for brevity here):

robrien@erwin:~/py/charmworld/ngrams$ ./bin/test charmworld/tests/test_search.py:TestReindex.test_reindexed_no_client_charms
Ran 1 test in 0.480s


Now I have no index or ingested charms:

robrien@erwin:~/py/charmworld/ngrams$ curl localhost:9200/charms/charm/_count?q=* && echo
{"error":"IndexMissingException[[charms] missing]","status":404}

I run es-update:

robrien@erwin:~/py/charmworld/ngrams$ ./bin/es-update

I now have an index but no charms:

robrien@erwin:~/py/charmworld/ngrams$ curl localhost:9200/charms/charm/_count?q=* && echo

Now I need to re-ingest to get charms again which takes a non-trivial amount of time.

Revision history for this message
Juju Gui Bot (juju-gui-bot) wrote :

Fix committed into lp:charmworld at revision 510, scheduled for release in charmworld, milestone Unknown

Changed in charmworld:
status: New → Fix Committed
Changed in charmworld:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.