Add an API for bundle metrics

Add an API for bundle metrics



https://code.launchpad.net/~benji/charmworld/bundle-deploy-metric-api/+merge/194564

(do not edit description out of merge proposal)

[benji, 2013-11-08 18:36:26]

Please take a look.

[benji, 2013-11-08 18:38:32]

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

https://codereview.appspot.com/23740043/diff/1/charmworld/models.py#newcode1
charmworld/models.py:1: # Copyright 2012, 2013 Marco Ceppi, Canonical Ltd.  This
software is
All the changes in this file are actually from my previous branch that was
apparently not merged. They have been reviewed previously.

https://codereview.appspot.com/23740043/diff/1/charmworld/tests/test_models.py
File charmworld/tests/test_models.py (right):

https://codereview.appspot.com/23740043/diff/1/charmworld/tests/test_models.p...
charmworld/tests/test_models.py:1: # Copyright 2012, 2013 Marco Ceppi, Canonical
Ltd.  This software is
All the changes in this file are actually from my previous branch that was
apparently not merged.  They have been reviewed previously.

[bac, 2013-11-08 19:03:29]

LGTMB

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

https://codereview.appspot.com/23740043/diff/1/charmworld/testing/factory.py#...
charmworld/testing/factory.py:365: makeBundle = make_bundle
You sure you'd not like to just fix them?

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

https://codereview.appspot.com/23740043/diff/1/charmworld/views/api/__init__....
charmworld/views/api/__init__.py:511: bundle_metric_types = ('deployments')
I think you meant for this to be a tuple.  One of my least favorite warts.

https://codereview.appspot.com/23740043/diff/1/charmworld/views/api/__init__....
charmworld/views/api/__init__.py:521: if metric_name not in
self.bundle_metric_types:
Huh, it works anyway.

https://codereview.appspot.com/23740043/diff/1/charmworld/views/tests/test_me...
File charmworld/views/tests/test_metrics_api.py (right):

https://codereview.appspot.com/23740043/diff/1/charmworld/views/tests/test_me...
charmworld/views/tests/test_metrics_api.py:2: 
del

https://codereview.appspot.com/23740043/diff/1/charmworld/views/tests/test_me...
charmworld/views/tests/test_metrics_api.py:20: 
Thank you for not piling onto the other test files.

[rharding, 2013-11-08 20:03:11]

+1 on brad's point of the tuple.

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

https://codereview.appspot.com/23740043/diff/1/charmworld/views/api/__init__....
charmworld/views/api/__init__.py:521: if metric_name not in
self.bundle_metric_types:
On 2013/11/08 19:03:29, bac wrote:
> Huh, it works anyway.


metric types will be the string and you can check if one string is in another.
It just so happens to work because deployments is in the string deployments.

It needs a comma up above.

[benji, 2013-11-12 15:59:42]

I forgot to publish my draft comments on Friday.  I know everyone has been
waiting patiently for them.

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

https://codereview.appspot.com/23740043/diff/1/charmworld/testing/factory.py#...
charmworld/testing/factory.py:365: makeBundle = make_bundle
On 2013/11/08 19:03:29, bac wrote:
> You sure you'd not like to just fix them?

I'd like to... but I figured a mechanical branch later would be better.

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

https://codereview.appspot.com/23740043/diff/1/charmworld/views/api/__init__....
charmworld/views/api/__init__.py:511: bundle_metric_types = ('deployments')
Good eye.  Fixed.

https://codereview.appspot.com/23740043/diff/1/charmworld/views/api/__init__....
charmworld/views/api/__init__.py:521: if metric_name not in
self.bundle_metric_types:
On 2013/11/08 19:03:29, bac wrote:
> Huh, it works anyway.

Well, I learned something new today.

https://codereview.appspot.com/23740043/diff/1/charmworld/views/tests/test_me...
File charmworld/views/tests/test_metrics_api.py (right):

https://codereview.appspot.com/23740043/diff/1/charmworld/views/tests/test_me...
charmworld/views/tests/test_metrics_api.py:2: 
On 2013/11/08 19:03:29, bac wrote:
> del

Done.

https://codereview.appspot.com/23740043/diff/1/charmworld/views/tests/test_me...
charmworld/views/tests/test_metrics_api.py:20: 
On 2013/11/08 19:03:29, bac wrote:
> Thank you for not piling onto the other test files.

Yeah, we need to start breaking those files apart.  A few are way over the
weight limit.