Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/ugly-add-node-errors into
lp:maas.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~jtv/maas/ugly-add-node-errors/+merge/99271
We now have user-friendly Fault exceptions for provisioning errors. But since
Fault is not in the list of exception types that our exception middleware knows
how to represent in a friendly manner, the UI will just show an ugly repr() of
the exception.
One way of solving this would have been to extend the exceptions middleware to
deal specially with Faults. But then what of unexpected, unknown instances of
Fault? We might want the full information (fault code, traceback) for
debugging.
So instead, I changed the code that represents Faults during provisioning in a
user-friendly way. It now turns them into MAASAPIExceptions, which get
rendered as user-friendly text.
Note that unfortunately, I can now no longer use regex matching in
ExpectedExceptions for these cases. This is because the exception string may
span multiple lines (the ugly repr() would turn newlines into even uglier “\n”)
and even in multi-line mode, I can't get ExpectedException to accept that I
matched the string. I suppose it wants every line to match or something. But
luckily the same test case has more detailed text matching checks for each of
these exceptions. The two tests that lose string matching are there to test
integration of exception catching with exception conversion, not so much to
test the user-friendly exception representation per se.
--
https://code.launchpad.net/~jtv/maas/ugly-add-node-errors/+merge/99271
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~jtv/maas/ugly-add-node-errors into lp:maas.
=== modified file 'src/maasserver/provisioning.py'
--- src/maasserver/provisioning.py 2012-03-23 08:19:11 +0000
+++ src/maasserver/provisioning.py 2012-03-26 09:08:36 +0000
@@ -26,6 +26,7 @@
post_save,
)
from django.dispatch import receiver
+from maasserver.exceptions import MAASAPIException
from maasserver.models import (
Config,
MACAddress,
@@ -85,7 +86,7 @@
Otherwise, this returns None and the original exception should be
re-raised. (This is left to the caller in order to minimize
erosion of the backtrace).
- :rtype: :class:`Exception`, or None.
+ :rtype: :class:`MAASAPIException`, or None.
"""
params = {
'fault_code': fault.faultCode,
@@ -95,8 +96,8 @@
if user_friendly_text is None:
return None
else:
- user_friendly_text = dedent(user_friendly_text.lstrip('\n') % params)
- return xmlrpclib.Fault(fault.faultCode, user_friendly_text)
+ return MAASAPIException(dedent(
+ user_friendly_text.lstrip('\n') % params))
class ProvisioningCaller:
=== modified file 'src/maasserver/tests/test_provisioning.py'
--- src/maasserver/tests/test_provisioning.py 2012-03-26 05:07:12 +0000
+++ src/maasserver/tests/test_provisioning.py 2012-03-26 09:08:36 +0000
@@ -15,6 +15,7 @@
from xmlrpclib import Fault
from maasserver import provisioning
+from maasserver.exceptions import MAASAPIException
from maasserver.models import (
ARCHITECTURE,
Config,
@@ -127,21 +128,18 @@
raise Fault(PSERV_FAULT.NO_SUCH_PROFILE, "Unknown profile.")
self.papi.patch('add_node', raise_missing_profile)
- expectation = ExpectedException(
- Fault, value_re='.*maas-import-isos.*')
- with expectation:
+ with ExpectedException(MAASAPIException):
node = factory.make_node(architecture='amd32k')
provisioning.provision_post_save_Node(
sender=Node, instance=node, created=True)
def test_provision_post_save_Node_returns_other_pserv_faults(self):
- error_text = factory.getRandomString()
def raise_fault(*args, **kwargs):
- raise Fault(PSERV_FAULT.NO_COBBLER, error_text)
+ raise Fault(PSERV_FAULT.NO_COBBLER, factory.getRandomString())
self.papi.patch('add_node', raise_fault)
- with ExpectedException(Fault, ".*%s.*" % error_text):
+ with ExpectedException(MAASAPIException):
node = factory.make_node(architecture='amd32k')
provisioning.provision_post_save_Node(
sender=Node, instance=node, created=True)
@@ -243,7 +241,7 @@
self.papi.patch('add_node', raise_fault)
- with ExpectedException(Fault, ".*provisioning server.*"):
+ with ExpectedException(MAASAPIException, ".*provisioning server.*"):
self.papi.add_node('node', 'profile', 'power', {})
def test_provisioning_errors_are_reported_helpfully(self):
@@ -253,13 +251,13 @@
self.papi.patch('add_node', raise_provisioning_error)
- with ExpectedException(Fault, ".*Cobbler.*"):
+ with ExpectedException(MAASAPIException, ".*Cobbler.*"):
self.papi.add_node('node', 'profile', 'power', {})
def test_present_user_friendly_fault_describes_pserv_fault(self):
self.assertIn(
"provisioning server",
- present_user_friendly_fault(Fault(8002, 'error')).faultString)
+ present_user_friendly_fault(Fault(8002, 'error')).message)
def test_present_user_friendly_fault_covers_all_pserv_faults(self):
all_pserv_faults = set(map_enum(PSERV_FAULT).values())
@@ -271,26 +269,26 @@
for fault_code in map_enum(PSERV_FAULT).values():
original_fault = Fault(fault_code, fault_string)
new_fault = present_user_friendly_fault(original_fault)
- self.assertNotEqual(fault_string, new_fault.faultString)
+ self.assertNotEqual(fault_string, new_fault.message)
def test_present_user_friendly_fault_describes_cobbler_fault(self):
friendly_fault = present_user_friendly_fault(
Fault(PSERV_FAULT.NO_COBBLER, factory.getRandomString()))
- friendly_text = friendly_fault.faultString
+ friendly_text = friendly_fault.message
self.assertIn("unable to reach", friendly_text)
self.assertIn("Cobbler", friendly_text)
def test_present_user_friendly_fault_describes_cobbler_auth_fail(self):
friendly_fault = present_user_friendly_fault(
Fault(PSERV_FAULT.COBBLER_AUTH_FAILED, factory.getRandomString()))
- friendly_text = friendly_fault.faultString
+ friendly_text = friendly_fault.message
self.assertIn("failed to authenticate", friendly_text)
self.assertIn("Cobbler", friendly_text)
def test_present_user_friendly_fault_describes_cobbler_auth_error(self):
friendly_fault = present_user_friendly_fault(
Fault(PSERV_FAULT.COBBLER_AUTH_ERROR, factory.getRandomString()))
- friendly_text = friendly_fault.faultString
+ friendly_text = friendly_fault.message
self.assertIn("authentication token", friendly_text)
self.assertIn("Cobbler", friendly_text)
@@ -300,7 +298,7 @@
Fault(
PSERV_FAULT.NO_SUCH_PROFILE,
"invalid profile name: %s" % profile))
- friendly_text = friendly_fault.faultString
+ friendly_text = friendly_fault.message
self.assertIn(profile, friendly_text)
self.assertIn("maas-import-isos", friendly_text)
@@ -308,7 +306,7 @@
error_text = factory.getRandomString()
friendly_fault = present_user_friendly_fault(
Fault(PSERV_FAULT.GENERIC_COBBLER_ERROR, error_text))
- friendly_text = friendly_fault.faultString
+ friendly_text = friendly_fault.message
self.assertIn("Cobbler", friendly_text)
self.assertIn(error_text, friendly_text)
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
