Add support for proof'ing bundle relations.

Add support for proof'ing bundle relations.

- Update deployer to 0.2.5
- Update (temporarily) to dupe the old deployer functionality lost for the
models.py to work properly.
- Use the deployer to help resolve the relations into testable bits.
- Make sure the relations resolve to real charms in the bundle.
- Make sure that those relations are for valid interfaces on both end points.
- Drive by bug fix for the config data type validation, add missing 'passing'
test.

https://code.launchpad.net/~rharding/charmworld/proof-relations/+merge/193614

(do not edit description out of merge proposal)

[rharding, 2013-11-01 17:24:20]

httpie


 http --form POST http://charmworld:2464/api/3/bundle/proof deployer_file='{
"wiki": { "services": { "wiki": { "charm": "mediawiki", "num_units": 2,
"branch": "lp:charms/precise/mediawiki", "constraints": "mem=2" }, "haproxy": {
"options": { "request-limit": 32 }, "branch": "lp:charms/precise/haproxy" },
"db": { "charm": "mysql", "options": { "tuning-level": "fast" }, "branch":
"lp:charms/precise/mysql" }, "memcached": { "options": { "request-limit": 32 },
"branch": "lp:charms/precise/memcached" } }, "series": "precise", "promulgated":
true, "relations": [ [ "wiki:db", "memcached" ], [ "db", [ "haproxy", "db" ] ] ]
} }'

[rharding, 2013-11-01 19:13:43]

Please take a look.

[rharding, 2013-11-01 19:19:38]

Reviewer comments

https://codereview.appspot.com/20810043/diff/40001/charmworld/lib/cdeployer.py
File charmworld/lib/cdeployer.py (right):

https://codereview.appspot.com/20810043/diff/40001/charmworld/lib/cdeployer.p...
charmworld/lib/cdeployer.py:9: def parse_deployer(data, format='yaml'):
changed this to a function as requested as a drive by change.

https://codereview.appspot.com/20810043/diff/40001/charmworld/lib/cdeployer.p...
charmworld/lib/cdeployer.py:22: class CharmworldDeployment(Deployment):
needed to use the deployer helpers for resolving relations. Eventually this
should flesh out to use more and more of the deployment code vs our own.

https://codereview.appspot.com/20810043/diff/40001/charmworld/lib/cdeployer.p...
charmworld/lib/cdeployer.py:37: # XXX: This is copied old deployer code needed
to make the models
copied stuff from the 0.1.1 version. It needs to be updated to use the deployer
config object.

https://codereview.appspot.com/20810043/diff/40001/charmworld/lib/proof.py
File charmworld/lib/proof.py (right):

https://codereview.appspot.com/20810043/diff/40001/charmworld/lib/proof.py#ne...
charmworld/lib/proof.py:16: 'boolean': (bool,),
this is the drive by fix. type xxx will not be a string value like in JS. :/

https://codereview.appspot.com/20810043/diff/40001/charmworld/lib/proof.py#ne...
charmworld/lib/proof.py:51: def
verify_points_exist_and_same_type(check_requires, check_provides,
this is purely for the simple case where it's a fully qualified relation.

[myapp:db, mysql:db]

https://codereview.appspot.com/20810043/diff/40001/charmworld/lib/proof.py#ne...
charmworld/lib/proof.py:83: def get_list_of_shared_interfaces(service_one,
service_two):
this is used to help *find* a compatible interface between two services. You can
define things like

[myapp:db, mysql] and [myapp, mysql] and it needs to be able to say that they do
share two endpoints that implement the mysql interface and can be made
compatible.

https://codereview.appspot.com/20810043/diff/40001/charmworld/views/api/proof.py
File charmworld/views/api/proof.py (right):

https://codereview.appspot.com/20810043/diff/40001/charmworld/views/api/proof...
charmworld/views/api/proof.py:35: for relation_error in bundle['relations']:
update the output to include the relations errors if they're in the resulting
bundle_info block.

https://codereview.appspot.com/20810043/diff/40001/charmworld/views/api/proof...
charmworld/views/api/proof.py:84: relations = deployment.get_relations()
the bit of the deployer we're currently using to plan out the points to check.

https://codereview.appspot.com/20810043/diff/40001/charmworld/views/api/proof...
charmworld/views/api/proof.py:127: elif check_requires or check_provides:
this is we should check either the specific require or provides for the case of

[myapp:db, mysql] or [myapp, mysql:db]

[benji, 2013-11-01 20:33:34]

LGTM -- I had a few small comments and one refactoring for you to consider.

https://codereview.appspot.com/20810043/diff/40001/charmworld/lib/cdeployer.py
File charmworld/lib/cdeployer.py (right):

https://codereview.appspot.com/20810043/diff/40001/charmworld/lib/cdeployer.p...
charmworld/lib/cdeployer.py:1: """Helpers for dealing with deployer files."""
A newline after the docstring would help it stand out a bit more.

https://codereview.appspot.com/20810043/diff/40001/charmworld/lib/cdeployer.p...
charmworld/lib/cdeployer.py:37: # XXX: This is copied old deployer code needed
to make the models
It would be good to clarify that "this" means all the code from this point until
the end of the file.

https://codereview.appspot.com/20810043/diff/40001/charmworld/lib/cdeployer.p...
charmworld/lib/cdeployer.py:51: # Utils from deployer 1
This is redundant with the above comment, right?

https://codereview.appspot.com/20810043/diff/40001/charmworld/lib/proof.py
File charmworld/lib/proof.py (right):

https://codereview.appspot.com/20810043/diff/40001/charmworld/lib/proof.py#ne...
charmworld/lib/proof.py:51: def
verify_points_exist_and_same_type(check_requires, check_provides,
"endpoints" instead of just "points" would help reading comprehension.

https://codereview.appspot.com/20810043/diff/40001/charmworld/lib/proof.py#ne...
charmworld/lib/proof.py:80: return True
Since this can only return True or raise an exception, is the return value
necessary?

https://codereview.appspot.com/20810043/diff/40001/charmworld/lib/proof.py#ne...
charmworld/lib/proof.py:83: def get_list_of_shared_interfaces(service_one,
service_two):
I found a good refactoring of this function:

def find_common_interfaces(provides, requires):
    """Return the set of interfaces that are shared between two endpoints"""
    def extract_relation_types(endpoints):
        return set([relation['interface'] for relation in endpoints.values()])

    return extract_relation_types(provides) & extract_relation_types(requires)


def get_list_of_shared_interfaces(service_one, service_two):
    """Generate a set of compatible interface types between services."""
    return (
        find_common_interfaces(service_one.provides, service_two.requires) |
        find_common_interfaces(service_two.provides, service_one.requires))

I suggest renaming get_list_of_shared_interfaces because it does not return a
list.

https://codereview.appspot.com/20810043/diff/40001/charmworld/views/tests/tes...
File charmworld/views/tests/test_proof.py (right):

https://codereview.appspot.com/20810043/diff/40001/charmworld/views/tests/tes...
charmworld/views/tests/test_proof.py:219: bundle_base = """
It would probably be an improvement to put these in the module scope just so
they don't break up the test functions so much.  That or use the dedent function
so they can visually match the indention of their surrounding code.

[bac, 2013-11-01 20:41:52]

not a real review

https://codereview.appspot.com/20810043/diff/40001/charmworld/lib/proof.py
File charmworld/lib/proof.py (right):

https://codereview.appspot.com/20810043/diff/40001/charmworld/lib/proof.py#ne...
charmworld/lib/proof.py:46: # XXX: The code below should be ported over to the
deployer or proof libary
typo: library

[rharding, 2013-11-02 01:29:55]

Thanks so much for the review. Updates applied and inbound. Commends (mainly
agreement and thanks) below.

https://codereview.appspot.com/20810043/diff/40001/charmworld/lib/cdeployer.py
File charmworld/lib/cdeployer.py (right):

https://codereview.appspot.com/20810043/diff/40001/charmworld/lib/cdeployer.p...
charmworld/lib/cdeployer.py:51: # Utils from deployer 1
On 2013/11/01 20:33:34, benji wrote:
> This is redundant with the above comment, right?

Yea, this was just part of the copied code. I didn't go into it a bunch.

https://codereview.appspot.com/20810043/diff/40001/charmworld/lib/proof.py
File charmworld/lib/proof.py (right):

https://codereview.appspot.com/20810043/diff/40001/charmworld/lib/proof.py#ne...
charmworld/lib/proof.py:83: def get_list_of_shared_interfaces(service_one,
service_two):
On 2013/11/01 20:33:34, benji wrote:
> I found a good refactoring of this function:
> 
> def find_common_interfaces(provides, requires):
>     """Return the set of interfaces that are shared between two endpoints"""
>     def extract_relation_types(endpoints):
>         return set([relation['interface'] for relation in endpoints.values()])
> 
>     return extract_relation_types(provides) & extract_relation_types(requires)
> 
> 
> def get_list_of_shared_interfaces(service_one, service_two):
>     """Generate a set of compatible interface types between services."""
>     return (
>         find_common_interfaces(service_one.provides, service_two.requires) |
>         find_common_interfaces(service_two.provides, service_one.requires))
> 
> I suggest renaming get_list_of_shared_interfaces because it does not return a
> list.

Thank you very much for this. A LOT nicer.

https://codereview.appspot.com/20810043/diff/40001/charmworld/views/tests/tes...
File charmworld/views/tests/test_proof.py (right):

https://codereview.appspot.com/20810043/diff/40001/charmworld/views/tests/tes...
charmworld/views/tests/test_proof.py:219: bundle_base = """
On 2013/11/01 20:33:34, benji wrote:
> It would probably be an improvement to put these in the module scope just so
> they don't break up the test functions so much.  That or use the dedent
function
> so they can visually match the indention of their surrounding code.

Very cool, I hadn't seen dedent. This bugged me to no end, so glad to be able to
clean them up a bit.

[rharding, 2013-11-04 15:06:33]

Please take a look.