Charm Helpers

Merge lp:~michael.nelson/charm-helpers/relation-get-return-none-outside-relation-hook into lp:charm-helpers

  1. relation-get-return-none-outside-relation-hook
  2. Merge into devel
Proposed by Michael Nelson
Status: Merged
Approved by: Gareth Woolridge
Approved revision: 87
Merged at revision: 87
Proposed branch: lp:~michael.nelson/charm-helpers/relation-get-return-none-outside-relation-hook
Merge into: lp:charm-helpers
Diff against target: 68 lines (+27/-0)
3 files modified
README.test (+1/-0)
charmhelpers/core/hookenv.py (+5/-0)
tests/core/test_hookenv.py (+21/-0)
To merge this branch: bzr merge lp:~michael.nelson/charm-helpers/relation-get-return-none-outside-relation-hook
Reviewer Review Type Date Requested Status
Gareth Woolridge Approve
Review via email: mp+191946@code.launchpad.net

Commit message

relation_get() returns None outside relation hook (instead of error).

Description of the change

With recent versions of juju-core, relation_get() raises an error if called outside of a relation hook without a -r relation_id specified [1]. The current execution_environment helper (which is used lots in charm-helpers) calls relation_get().

The actual error corresponds to an exit 2 error status [2]. I've differentiated on the error code because other (more serious) errors returned by relation-get use 1 [3]

[1] https://pastebin.canonical.com/99283/ Note that relation-list --help says this explicitly while relation-get --help does not.
[2] https://pastebin.canonical.com/99281/
[3] https://pastebin.canonical.com/99282/

To post a comment you must log in.
Revision history for this message
Gareth Woolridge (moon127) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'README.test'
2--- README.test 2013-05-22 21:11:01 +0000
3+++ README.test 2013-10-21 08:36:48 +0000
4@@ -5,3 +5,4 @@
5 python-nose
6 python-mock
7 python-testtools
8+python-jinja2
9
10=== modified file 'charmhelpers/core/hookenv.py'
11--- charmhelpers/core/hookenv.py 2013-09-26 14:36:33 +0000
12+++ charmhelpers/core/hookenv.py 2013-10-21 08:36:48 +0000
13@@ -9,6 +9,7 @@
14 import yaml
15 import subprocess
16 import UserDict
17+from subprocess import CalledProcessError
18
19 CRITICAL = "CRITICAL"
20 ERROR = "ERROR"
21@@ -175,6 +176,10 @@
22 return json.loads(subprocess.check_output(_args))
23 except ValueError:
24 return None
25+ except CalledProcessError, e:
26+ if e.returncode == 2:
27+ return None
28+ raise
29
30
31 def relation_set(relation_id=None, relation_settings={}, **kwargs):
32
33=== modified file 'tests/core/test_hookenv.py'
34--- tests/core/test_hookenv.py 2013-08-07 17:05:39 +0000
35+++ tests/core/test_hookenv.py 2013-10-21 08:36:48 +0000
36@@ -1,4 +1,5 @@
37 import json
38+from subprocess import CalledProcessError
39
40 import cPickle as pickle
41 from mock import patch, call, mock_open
42@@ -584,6 +585,26 @@
43
44 self.assertIsNone(result)
45
46+ @patch('charmhelpers.core.hookenv.subprocess')
47+ def test_relation_get_calledprocesserror(self, mock_subprocess):
48+ """relation-get called outside a relation will errors without id."""
49+ mock_subprocess.check_output.side_effect = CalledProcessError(
50+ 2, '/foo/bin/relation-get'
51+ 'no relation id specified')
52+
53+ result = hookenv.relation_get()
54+
55+ self.assertIsNone(result)
56+
57+ @patch('charmhelpers.core.hookenv.subprocess')
58+ def test_relation_get_calledprocesserror_other(self, mock_subprocess):
59+ """relation-get can fail for other more serious errors."""
60+ mock_subprocess.check_output.side_effect = CalledProcessError(
61+ 1, '/foo/bin/relation-get'
62+ 'connection refused')
63+
64+ self.assertRaises(CalledProcessError, hookenv.relation_get)
65+
66 @patch('subprocess.check_output')
67 def test_gets_relation_with_scope(self, check_output):
68 check_output.return_value = json.dumps('bar')

Subscribers

People subscribed via source and target branches