|
|
|
Bundle proofing plus migration.
Bundles are only rejected if there are errors not just warnings.
A migration script is provided that will remove all existing bundles. The
good bundles will later be repopulated when ingest runs again.
https://code.launchpad.net/~bac/charmworld/allow-bundles-with-warnings/+merge/193992
(do not edit description out of merge proposal)
Patch Set 1 #
Total comments: 6
Patch Set 2 : Bundle proofing plus migration. #Patch Set 3 : Bundle proofing plus migration. #
Total messages: 6
|
bac
Please take a look.
|
12 years, 2 months ago (2013年11月05日 19:17:21 UTC) #1 | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Please take a look.
LGTM with a few comments added. Thank you! https://codereview.appspot.com/21790045/diff/1/charmworld/jobs/ingest.py File charmworld/jobs/ingest.py (right): https://codereview.appspot.com/21790045/diff/1/charmworld/jobs/ingest.py#newc... charmworld/jobs/ingest.py:73: CHARMTOOL_ERROR_CODE = 200 This is so seemingly arbitrary that it is calling out to me for some comments explaining the purpose, context, and meaning. Could be here, or down in the "proof" method. https://codereview.appspot.com/21790045/diff/1/charmworld/migrations/migrate.py File charmworld/migrations/migrate.py (right): https://codereview.appspot.com/21790045/diff/1/charmworld/migrations/migrate.... charmworld/migrations/migrate.py:38: db is the pymongo db instance for our datastore. Charms are in db.charms A docstring! Yay! Don't leave it all by its lonesome: explain what the heck the index_client is! https://codereview.appspot.com/21790045/diff/1/charmworld/migrations/versions... File charmworld/migrations/versions/021_drop_unproofed_bundles.py (right): https://codereview.appspot.com/21790045/diff/1/charmworld/migrations/versions... charmworld/migrations/versions/021_drop_unproofed_bundles.py:12: db.bundles.drop() I like that kind of simplicity.
Please take a look. https://codereview.appspot.com/21790045/diff/1/charmworld/jobs/ingest.py File charmworld/jobs/ingest.py (right): https://codereview.appspot.com/21790045/diff/1/charmworld/jobs/ingest.py#newc... charmworld/jobs/ingest.py:73: CHARMTOOL_ERROR_CODE = 200 On 2013年11月05日 19:49:05, gary.poster wrote: > This is so seemingly arbitrary that it is calling out to me for some comments > explaining the purpose, context, and meaning. Could be here, or down in the > "proof" method. Done. https://codereview.appspot.com/21790045/diff/1/charmworld/migrations/migrate.py File charmworld/migrations/migrate.py (right): https://codereview.appspot.com/21790045/diff/1/charmworld/migrations/migrate.... charmworld/migrations/migrate.py:38: db is the pymongo db instance for our datastore. Charms are in db.charms On 2013年11月05日 19:49:05, gary.poster wrote: > A docstring! Yay! Don't leave it all by its lonesome: explain what the heck > the index_client is! Done. https://codereview.appspot.com/21790045/diff/1/charmworld/migrations/versions... File charmworld/migrations/versions/021_drop_unproofed_bundles.py (right): https://codereview.appspot.com/21790045/diff/1/charmworld/migrations/versions... charmworld/migrations/versions/021_drop_unproofed_bundles.py:12: db.bundles.drop() Indeed.
Thank you Brad. LGTM.