Add an index for db.charms.

Add an index for db.charms.

Without the proper index, sorting mongo results fail.

* Add setup_mongo_indices and make it idempotent.
* Lots of tests.
* Change migration 'initialize' to call setup_mongo_indices.
* Add a migration for existing dbs.

* Drive by ugliness

The migration scripts, even long-lived ones like test_migrations.py, were
quite dirty lint-wise.  Turns out the Makefile was excluding them explicitly.
Changed the lint target and then had to fix the old scripts.

https://code.launchpad.net/~bac/charmworld/bug-1278222/+merge/205655

(do not edit description out of merge proposal)

[bac, 2014-02-10 21:38:51]

Please take a look.

[j.c.sackett, 2014-02-10 22:28:57]

https://codereview.appspot.com/61720043/diff/1/charmworld/migrations/migrate.py
File charmworld/migrations/migrate.py (right):

https://codereview.appspot.com/61720043/diff/1/charmworld/migrations/migrate....
charmworld/migrations/migrate.py:33: SCRIPT_TEMPLATE = '''\
The trailing slash here shouldn't be necessary; triple single quotes is as valid
for a multiline string as double quotes.

https://codereview.appspot.com/61720043/diff/1/charmworld/migrations/migrate....
charmworld/migrations/migrate.py:57: '''
Should this be part of the template? Most migrations are *not* exoduses.

Maybe add a note that this can probably be deleted, for devs not familiar with
migration vs exodus?

It's also worth noting that as it's currently set, this is not background index
creation, so it's going to block db operations, so when your changes go live,
watch for downtime.

https://codereview.appspot.com/61720043/diff/1/charmworld/migrations/migrate....
charmworld/migrations/migrate.py:87: db.charms.create_index(
Does pymongo expose ensureIndex? That only creates the index if it doesn't
already exist, IIRC.

https://codereview.appspot.com/61720043/diff/1/charmworld/migrations/migrate....
charmworld/migrations/migrate.py:192: setup_mongo_indices(datastore.db)
One place we may see issues is if we deploy charmworld anew somewhere,
initialization will fire as part of juju operations, and an ingest will happen
somewhere in there too--that ingest will probably be fine (index creation on no
data shouldn't take any time), but it's worth being aware of.

https://codereview.appspot.com/61720043/diff/1/charmworld/migrations/versions...
File charmworld/migrations/versions/024_create_charms_index.py (right):

https://codereview.appspot.com/61720043/diff/1/charmworld/migrations/versions...
charmworld/migrations/versions/024_create_charms_index.py:13:
setup_mongo_indices(db)
This migration is going to be run more or less as soon as you deploy this
revision, and it's worth noting that here all db ops on charmworld will go down.

I don't know that background creation will be better (or is exposed via
pymongo); that slows down performance instead of halting it, but the mongo docs
suggest with lots of data you could see hours and hours of performance
degradation. You'll probably want to see how long staging is unresponsive to get
an idea of when you should schedule a deploy to production.

https://codereview.appspot.com/61720043/diff/1/charmworld/testing/factory.py
File charmworld/testing/factory.py (right):

https://codereview.appspot.com/61720043/diff/1/charmworld/testing/factory.py#...
charmworld/testing/factory.py:323: """Make a bunch of charms.  Overwrite"""
Overwrite what?

[j.c.sackett, 2014-02-10 22:29:54]

LGTM, with the concerns on deploy and minor quibbles above.

[bac, 2014-02-11 15:40:59]

Please take a look.

https://codereview.appspot.com/61720043/diff/1/charmworld/migrations/migrate.py
File charmworld/migrations/migrate.py (right):

https://codereview.appspot.com/61720043/diff/1/charmworld/migrations/migrate....
charmworld/migrations/migrate.py:33: SCRIPT_TEMPLATE = '''\
Without the backslash, the first line consists of just \n.  I used the backslash
to suppress the leading blank line.

https://codereview.appspot.com/61720043/diff/1/charmworld/migrations/migrate....
charmworld/migrations/migrate.py:57: '''
I added the commented out template just so future migrations would have the
required function name, signature, and documented parameters.  I can add more
explanation about when and why to use an update versus an exodus but I don't see
the harm in including it.  The fact it is commented out gives a hint to its
lesser importance relative to the update.

https://codereview.appspot.com/61720043/diff/1/charmworld/migrations/migrate....
charmworld/migrations/migrate.py:87: db.charms.create_index(
It does and using it allows me to get rid of the existence test.  Thanks.

https://codereview.appspot.com/61720043/diff/1/charmworld/migrations/migrate....
charmworld/migrations/migrate.py:192: setup_mongo_indices(datastore.db)
I think it is ideal to establish the index before data are loaded.  In that
instance the index is trivially updated as new data are added to the database.

The expensive maneuver is adding an index post-hoc and having mongo
retroactively calculate the index.

https://codereview.appspot.com/61720043/diff/1/charmworld/migrations/versions...
File charmworld/migrations/versions/024_create_charms_index.py (right):

https://codereview.appspot.com/61720043/diff/1/charmworld/migrations/versions...
charmworld/migrations/versions/024_create_charms_index.py:13:
setup_mongo_indices(db)
I've changed it to an exodus, so it can be long-running without locking up the
db.

https://codereview.appspot.com/61720043/diff/1/charmworld/testing/factory.py
File charmworld/testing/factory.py (right):

https://codereview.appspot.com/61720043/diff/1/charmworld/testing/factory.py#...
charmworld/testing/factory.py:323: """Make a bunch of charms.  Overwrite"""
On 2014/02/10 22:28:57, j.c.sackett wrote:
> Overwrite what?

Done.