Juju Charms Collection
juju-gui package

Merge lp:~bac/charms/precise/juju-gui/enverror into lp:~juju-gui/charms/precise/juju-gui/trunk

  1. Precise Pangolin (12.04)
  2. enverror
  3. Merge into trunk
Proposed by Brad Crittenden
Status: Merged
Merged at revision: 141
Proposed branch: lp:~bac/charms/precise/juju-gui/enverror
Merge into: lp:~juju-gui/charms/precise/juju-gui/trunk
Diff against target: 75 lines (+19/-5)
4 files modified
revision (+1/-1)
server/guiserver/bundles/utils.py (+5/-2)
server/guiserver/tests/bundles/test_base.py (+1/-1)
server/guiserver/tests/bundles/test_utils.py (+12/-1)
To merge this branch: bzr merge lp:~bac/charms/precise/juju-gui/enverror
Reviewer Review Type Date Requested Status
charmers Pending
Review via email: mp+196966@code.launchpad.net

Description of the change

Extract the message from EnvError on deploy err.

https://codereview.appspot.com/34270044/

To post a comment you must log in.
Revision history for this message
Brad Crittenden (bac) wrote :
Download full text (3.8 KiB)

Reviewers: mp+196966_code.launchpad.net,

Message:
Please take a look.

Description:
Extract the message from EnvError on deploy err.

https://code.launchpad.net/~bac/charms/precise/juju-gui/enverror/+merge/196966

(do not edit description out of merge proposal)

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

Affected files (+21, -5 lines):
   A [revision details]
   M revision
   M server/guiserver/bundles/utils.py
   M server/guiserver/tests/bundles/test_base.py
   M server/guiserver/tests/bundles/test_utils.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: <email address hidden>
+New revision: <email address hidden>

Index: revision
=== modified file 'revision'
--- revision 2013-11-14 11:25:13 +0000
+++ revision 2013-11-27 19:26:26 +0000
@@ -1,1 +1,1 @@
-100
+101

Index: server/guiserver/bundles/utils.py
=== modified file 'server/guiserver/bundles/utils.py'
--- server/guiserver/bundles/utils.py 2013-11-19 14:56:29 +0000
+++ server/guiserver/bundles/utils.py 2013-11-27 19:25:54 +0000
@@ -30,7 +30,7 @@
  from tornado.httpclient import AsyncHTTPClient

  from guiserver.watchers import AsyncWatcher
-
+from jujuclient import EnvError

  # Change statuses.
  SCHEDULED = 'scheduled'
@@ -73,7 +73,10 @@
      """
      logging.error('error deploying the bundle')
      logging.error('error type: {}'.format(type(exception)))
- message = str(exception).strip()
+ if isinstance(exception, EnvError):
+ message = exception.message.strip()
+ else:
+ message = str(exception).strip()
      if message:
          logging.error('error message: {}'.format(message))
      else:

Index: server/guiserver/tests/bundles/test_base.py
=== modified file 'server/guiserver/tests/bundles/test_base.py'
--- server/guiserver/tests/bundles/test_base.py 2013-11-19 15:28:56 +0000
+++ server/guiserver/tests/bundles/test_base.py 2013-11-27 19:25:54 +0000
@@ -250,7 +250,7 @@
          expected = {
              'DeploymentId': 0,
              'Status': utils.COMPLETED,
- 'Error': "<Env Error - Details:\n { 'Error': 'bad wolf'}\n
>",
 + 'Error': "bad wolf",
              'Time': 42,
          }
          self.assertEqual(expected, status[0])

Index: server/guiserver/tests/bundles/test_utils.py
=== modified file 'server/guiserver/tests/bundles/test_utils.py'
--- server/guiserver/tests/bundles/test_utils.py 2013-11-19 15:28:56 +0000
+++ server/guiserver/tests/bundles/test_utils.py 2013-11-27 19:28:03 +0000
@@ -32,7 +32,7 @@
  from guiserver import watchers
  from guiserver.bundles import utils
  from guiserver.tests import helpers
-
+from jujuclient import EnvError

  mock_time = mock.patch('time.time', mock.Mock(return_value=12345))

@@ -93,6 +93,17 @@
                  error = utils.message_from_error(ValueError('bad wolf'))
          self.assertEqual('bad wolf', error)

+ def test_env_error_extracted(self):
+ # An EnvError as returned from the Go environment is not suitable
for
+ # display to t...

Read more...

Revision history for this message
Brad Crittenden (bac) wrote :

To QA:

The charm needs to have the juju-gui trunk. I tried just setting
juju-gui-source but ran into all kinds of problems.
What I eventually did was make a new release in juju-gui and copy it
over to my charm '/releases' directory. That's what I describe here.

In juju-gui:
BRANCH_IS_GOOD=1 make distfile
Note the new tar ball created.

0) Grab the branch: lp:~bac/charms/precise/juju-gui/enverror
0.1) Copy tarball from juju-gui to the charm releases directory.
1) juju switch local
2) sudo juju bootstrap
3) make deploy

Now go to the GUI. Search for 'owner: bac'. Drag bac's Jenkins bundle
onto the canvas. After a short while you should get a notification
which says:

Updated status for deployment id: 0

The deployment errored: json: cannot unmarshal string into Go value of
type uint64

https://codereview.appspot.com/34270044/diff/1/server/guiserver/tests/bundles/test_base.py
File server/guiserver/tests/bundles/test_base.py (right):

https://codereview.appspot.com/34270044/diff/1/server/guiserver/tests/bundles/test_base.py#newcode253
server/guiserver/tests/bundles/test_base.py:253: 'Error': "bad wolf",
The returned error is now more concise.

https://codereview.appspot.com/34270044/

Revision history for this message
Gary Poster (gary) wrote :

LGTM and QA good. Very nice that the fix could be so simple.

(Separately, I wish the errors were easier to spot, but that's a
different problem.)

Thank you!

https://codereview.appspot.com/34270044/

Revision history for this message
Brad Crittenden (bac) wrote :

*** Submitted:

Extract the message from EnvError on deploy err.

R=gary.poster
CC=
https://codereview.appspot.com/34270044

https://codereview.appspot.com/34270044/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'revision'
--- revision 2013-11-14 11:25:13 +0000
+++ revision 2013-11-27 19:32:10 +0000
@@ -1,1 +1,1 @@
11001101
22
=== modified file 'server/guiserver/bundles/utils.py'
--- server/guiserver/bundles/utils.py 2013-11-19 14:56:29 +0000
+++ server/guiserver/bundles/utils.py 2013-11-27 19:32:10 +0000
@@ -30,7 +30,7 @@
30from tornado.httpclient import AsyncHTTPClient30from tornado.httpclient import AsyncHTTPClient
3131
32from guiserver.watchers import AsyncWatcher32from guiserver.watchers import AsyncWatcher
3333from jujuclient import EnvError
3434
35# Change statuses.35# Change statuses.
36SCHEDULED = 'scheduled'36SCHEDULED = 'scheduled'
@@ -73,7 +73,10 @@
73 """73 """
74 logging.error('error deploying the bundle')74 logging.error('error deploying the bundle')
75 logging.error('error type: {}'.format(type(exception)))75 logging.error('error type: {}'.format(type(exception)))
76 message = str(exception).strip()76 if isinstance(exception, EnvError):
77 message = exception.message.strip()
78 else:
79 message = str(exception).strip()
77 if message:80 if message:
78 logging.error('error message: {}'.format(message))81 logging.error('error message: {}'.format(message))
79 else:82 else:
8083
=== modified file 'server/guiserver/tests/bundles/test_base.py'
--- server/guiserver/tests/bundles/test_base.py 2013-11-19 15:28:56 +0000
+++ server/guiserver/tests/bundles/test_base.py 2013-11-27 19:32:10 +0000
@@ -250,7 +250,7 @@
250 expected = {250 expected = {
251 'DeploymentId': 0,251 'DeploymentId': 0,
252 'Status': utils.COMPLETED,252 'Status': utils.COMPLETED,
253 'Error': "<Env Error - Details:\n { 'Error': 'bad wolf'}\n >",253 'Error': "bad wolf",
254 'Time': 42,254 'Time': 42,
255 }255 }
256 self.assertEqual(expected, status[0])256 self.assertEqual(expected, status[0])
257257
=== modified file 'server/guiserver/tests/bundles/test_utils.py'
--- server/guiserver/tests/bundles/test_utils.py 2013-11-19 15:28:56 +0000
+++ server/guiserver/tests/bundles/test_utils.py 2013-11-27 19:32:10 +0000
@@ -32,7 +32,7 @@
32from guiserver import watchers32from guiserver import watchers
33from guiserver.bundles import utils33from guiserver.bundles import utils
34from guiserver.tests import helpers34from guiserver.tests import helpers
3535from jujuclient import EnvError
3636
37mock_time = mock.patch('time.time', mock.Mock(return_value=12345))37mock_time = mock.patch('time.time', mock.Mock(return_value=12345))
3838
@@ -93,6 +93,17 @@
93 error = utils.message_from_error(ValueError('bad wolf'))93 error = utils.message_from_error(ValueError('bad wolf'))
94 self.assertEqual('bad wolf', error)94 self.assertEqual('bad wolf', error)
9595
96 def test_env_error_extracted(self):
97 # An EnvError as returned from the Go environment is not suitable for
98 # display to the user. The Error field is extracted and returned.
99 expected_type = "error type: <class 'jujuclient.EnvError'>"
100 expected_message = 'error message: cannot parse json'
101 with ExpectLog('', expected_type, required=True):
102 with ExpectLog('', expected_message, required=True):
103 exception = EnvError({'Error': 'cannot parse json'})
104 error = utils.message_from_error(exception)
105 self.assertEqual('cannot parse json', error)
106
96 def test_without_message(self):107 def test_without_message(self):
97 # A placeholder message is returned.108 # A placeholder message is returned.
98 expected_type = "error type: <type 'exceptions.SystemExit'>"109 expected_type = "error type: <type 'exceptions.SystemExit'>"

Subscribers

People subscribed via source and target branches