Refactory the proof api to be it's own module.

Refactory the proof api to be it's own module.

- Break out the proof api bits into a new api module vs the same api file.
- Move tests and fix to match
- Remove the model in the lib bits and try to keep things in views/models.

https://code.launchpad.net/~rharding/charmworld/proof-config2/+merge/192243

(do not edit description out of merge proposal)

[rharding, 2013-10-22 21:27:34]

Reviewer comments

https://codereview.appspot.com/15210050/diff/1/charmworld/lib/proof.py
File charmworld/lib/proof.py (left):

https://codereview.appspot.com/15210050/diff/1/charmworld/lib/proof.py#oldcode7
charmworld/lib/proof.py:7: )
model code moved from the lib and moved into a view.

https://codereview.appspot.com/15210050/diff/1/charmworld/lib/proof.py#oldcode17
charmworld/lib/proof.py:17: class BundleProof(object):
this is now part new model Deployer (since it's multiple bundles) and part view
work now.

https://codereview.appspot.com/15210050/diff/1/charmworld/lib/tests/test_proo...
File charmworld/lib/tests/test_proof.py (left):

https://codereview.appspot.com/15210050/diff/1/charmworld/lib/tests/test_proo...
charmworld/lib/tests/test_proof.py:16: class TestBundleProof(TestCase):
this is no longer the job of the lib, but of the model/view so tests are moved
there.

https://codereview.appspot.com/15210050/diff/1/charmworld/lib/tests/test_proo...
File charmworld/lib/tests/test_proof.py (right):

https://codereview.appspot.com/15210050/diff/1/charmworld/lib/tests/test_proo...
charmworld/lib/tests/test_proof.py:63: """Bundles might attempt to set config
values that do not exist"""
drive-by

https://codereview.appspot.com/15210050/diff/1/charmworld/models.py
File charmworld/models.py (right):

https://codereview.appspot.com/15210050/diff/1/charmworld/models.py#newcode1449
charmworld/models.py:1449: class DeployerParser(object):
the "can I parse it" is part of a Deployer model-ish work and removed from the
lib. This will/can grow to support different formats.

https://codereview.appspot.com/15210050/diff/1/charmworld/views/api/__init__.py
File charmworld/views/api/__init__.py (left):

https://codereview.appspot.com/15210050/diff/1/charmworld/views/api/__init__....
charmworld/views/api/__init__.py:52: def json_response(status, value,
headers=[]):
we've made the api.py into a python module and moved this into a view/utils to
assist in sharing across multiple view files.

https://codereview.appspot.com/15210050/diff/1/charmworld/views/api/__init__....
charmworld/views/api/__init__.py:497: def _proof_bundle(self, request):
and the proof code is moved to views/api/proof.py

https://codereview.appspot.com/15210050/diff/1/charmworld/views/api/__init__.py
File charmworld/views/api/__init__.py (right):

https://codereview.appspot.com/15210050/diff/1/charmworld/views/api/__init__....
charmworld/views/api/__init__.py:509: return proof_deployer(self.request)
this is really a deployer check vs a bundle until we start looping through the
bundles in a deployer file

https://codereview.appspot.com/15210050/diff/1/charmworld/views/tests/test_ap...
File charmworld/views/tests/test_api.py (right):

https://codereview.appspot.com/15210050/diff/1/charmworld/views/tests/test_ap...
charmworld/views/tests/test_api.py:871: class TestAPI3Bundles(TestAPIBundles,
API3Mixin, TestBundleProof):
the api needs to pull in the tests for the api/proof.py file.

https://codereview.appspot.com/15210050/diff/1/charmworld/views/utils.py
File charmworld/views/utils.py (right):

https://codereview.appspot.com/15210050/diff/1/charmworld/views/utils.py#newc...
charmworld/views/utils.py:5: def json_response(status, value, headers=[]):
moved to share between api/__init__.py and api/proof.py

[rharding, 2013-10-22 21:32:56]

Please take a look.

[rharding, 2013-10-22 22:02:45]

Please take a look.

[bac, 2013-10-22 22:40:33]

LGTM with some requested changes.  Some more refactoring to create api/charms.py
and api/bundles.py will be nice future work.

https://codereview.appspot.com/15210050/diff/210001/charmworld/lib/tests/test...
File charmworld/lib/tests/test_proof.py (right):

https://codereview.appspot.com/15210050/diff/210001/charmworld/lib/tests/test...
charmworld/lib/tests/test_proof.py:63: """Bundles might attempt to set config
values that do not exist"""
PunctuateMe

https://codereview.appspot.com/15210050/diff/210001/charmworld/models.py
File charmworld/models.py (right):

https://codereview.appspot.com/15210050/diff/210001/charmworld/models.py#newc...
charmworld/models.py:1449: class DeployerParser(object):
Having this simple parser as a class seems like overkill to me.  I think it'd be
better as a function in utils instead.

https://codereview.appspot.com/15210050/diff/210001/charmworld/views/api/proo...
File charmworld/views/api/proof.py (right):

https://codereview.appspot.com/15210050/diff/210001/charmworld/views/api/proo...
charmworld/views/api/proof.py:85: # The top level is for general deployer file
errors.
Either s/provided// or s/bundle provided/provided bundle/

https://codereview.appspot.com/15210050/diff/210001/charmworld/views/tests/te...
File charmworld/views/tests/test_api.py (right):

https://codereview.appspot.com/15210050/diff/210001/charmworld/views/tests/te...
charmworld/views/tests/test_api.py:875: class TestAPI2Bundles(TestAPIBundles,
API2Mixin, TestBundleProof):
I know this is already existing but why is there a TestAPI2Bundles?  APIv2 has
no bundles.

https://codereview.appspot.com/15210050/diff/210001/charmworld/views/tests/te...
File charmworld/views/tests/test_proof.py (right):

https://codereview.appspot.com/15210050/diff/210001/charmworld/views/tests/te...
charmworld/views/tests/test_proof.py:10: class TestBundleProof:
Since this class is only defined here but not run I'd make a very conspicuous
comment that this is a mixin class that is not run by itself.

https://codereview.appspot.com/15210050/diff/210001/charmworld/views/utils.py
File charmworld/views/utils.py (right):

https://codereview.appspot.com/15210050/diff/210001/charmworld/views/utils.py...
charmworld/views/utils.py:5: def json_response(status, value, headers=[]):
Thanks for moving this here

[rharding, 2013-10-22 22:55:29]

Thanks for the review. Updated and pushing. One comment below on why I'm not
going to remove the api3 tests on the api2 run below.

https://codereview.appspot.com/15210050/diff/210001/charmworld/views/tests/te...
File charmworld/views/tests/test_api.py (right):

https://codereview.appspot.com/15210050/diff/210001/charmworld/views/tests/te...
charmworld/views/tests/test_api.py:875: class TestAPI2Bundles(TestAPIBundles,
API2Mixin, TestBundleProof):
On 2013/10/22 22:40:33, bac wrote:
> I know this is already existing but why is there a TestAPI2Bundles?  APIv2 has
> no bundles.

I honestly wasn't sure why things were in both. I *think* it's because API2 is
created via extending API3. So we have to make sure tests work on both since
they're really a nested object.