Code review comment for lp:~rharding/charmworld/support-implicit-relations

Revision history for this message
Richard Harding (rharding) wrote :

Reviewers: mp+194535_code.launchpad.net,

Message:
Please take a look.

Description:
Allow implicit juju-* relations to be valid.

- https://juju.ubuntu.com/docs/authors-implicit-relations.html
- juju-* is a namespace for implicit juju relations all charms support.
This
adds a check for relations that start with juju- and auto approve them
in
proof.
- Charms are not allowed to create any relation of their own that start
with
juju-, so it's safe to assume anything that starts with this is a safe
implicit relation.

https://code.launchpad.net/~rharding/charmworld/support-implicit-relations/+merge/194535

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/23660043/

Affected files (+37, -0 lines):
   A [revision details]
   M charmworld/lib/proof.py
   M charmworld/lib/tests/test_proof.py

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: tarmac-20131108152057-vxjx6ibvpchmbuqw
+New revision: <email address hidden>

Index: charmworld/lib/proof.py
=== modified file 'charmworld/lib/proof.py'
--- charmworld/lib/proof.py 2013-11-06 19:10:38 +0000
+++ charmworld/lib/proof.py 2013-11-08 15:33:33 +0000
@@ -71,6 +71,15 @@
      if check_requires in requires and check_provides in provides:
          exist = True

+ # If the check type is a implicit interface then it's valid.
+ if check_requires and check_requires.startswith('juju-'):
+ return True
+
+ if check_provides and check_provides.startswith('juju-'):
+ return True
+
+ # If it's not implicit then continue to check that it's valid for the
two
+ # services in the relation.
      if check_requires and check_requires in requires:
          require_type = requires[check_requires]['interface']
      if check_provides and check_provides in provides:

Index: charmworld/lib/tests/test_proof.py
=== modified file 'charmworld/lib/tests/test_proof.py'
--- charmworld/lib/tests/test_proof.py 2013-11-06 19:17:39 +0000
+++ charmworld/lib/tests/test_proof.py 2013-11-08 15:33:33 +0000
@@ -322,3 +322,29 @@

          # There's no ProofError raised. Just an empty function return value
          self.assertTrue(res)
+
+ def test_implicit_relations_are_safe(self):
+ # relation = ['wiki:juju-info', 'haproxy:juju-info']
+ check_requires = 'juju-info'
+ check_provides = 'juju-info'
+
+ # Note that none of the services defined a juju-info interface.
+ requires = {
+ 'website': {
+ 'interface': 'http'
+ }
+ }
+ provides = {
+ 'cache': {
+ 'interface': 'http'
+ }
+ }
+
+ res = verify_points_exist_and_same_type(
+ check_requires,
+ check_provides,
+ requires,
+ provides)
+
+ # There's no ProofError raised. Just an empty function return value
+ self.assertTrue(res)

« Back to merge proposal