Code review comment for lp:~bac/charms/precise/juju-gui/enverror

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 the user. The Error field is extracted and returned.
+ expected_type = "error type: <class 'jujuclient.EnvError'>"
+ expected_message = 'error message: cannot parse json'
+ with ExpectLog('', expected_type, required=True):
+ with ExpectLog('', expected_message, required=True):
+ exception = EnvError({'Error': 'cannot parse json'})
+ error = utils.message_from_error(exception)
+ self.assertEqual('cannot parse json', error)
+
      def test_without_message(self):
          # A placeholder message is returned.
          expected_type = "error type: <type 'exceptions.SystemExit'>"