launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #07076
[Merge] lp:~rvb/maas/one-off-messages-shorter into lp:maas
Raphaël Badin has proposed merging lp:~rvb/maas/one-off-messages-shorter into lp:maas.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~rvb/maas/one-off-messages-shorter/+merge/101506
This branch refactor the error reporting code in provisioning.py to add shorter error messages that will be used for one-off error messages. The more detailed error messages will be used for long-lasting warnings.
== Pre-implementation notes ==
Had a talk with jtv about that. The detailed error messages were introduced when we had no way to display long-lasting warnings. Now that we have, the one-off message should be shorter.
--
https://code.launchpad.net/~rvb/maas/one-off-messages-shorter/+merge/101506
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/one-off-messages-shorter into lp:maas.
=== modified file 'src/maasserver/provisioning.py'
--- src/maasserver/provisioning.py 2012-04-05 20:22:48 +0000
+++ src/maasserver/provisioning.py 2012-04-11 08:28:18 +0000
@@ -11,9 +11,11 @@
__metaclass__ = type
__all__ = [
'get_provisioning_api_proxy',
+ 'present_detailed_user_friendly_fault',
'ProvisioningProxy',
]
+from functools import partial
from logging import getLogger
from textwrap import dedent
from urllib import urlencode
@@ -36,8 +38,9 @@
from provisioningserver.enum import PSERV_FAULT
import yaml
-# Presentation templates for various provisioning faults.
-PRESENTATIONS = {
+# Presentation templates for various provisioning faults (will be used
+# for long-lasting warnings about failing components).
+DETAILED_PRESENTATIONS = {
PSERV_FAULT.NO_COBBLER: """
The provisioning server was unable to reach the Cobbler service:
%(fault_string)s
@@ -78,12 +81,37 @@
""",
}
-
-def present_user_friendly_fault(fault):
+# Shorter presentation templates for various provisioning faults (will
+# be used for one-off messages).
+SHORT_PRESENTATIONS = {
+ PSERV_FAULT.NO_COBBLER: """
+ Unable to reach the Cobbler server.
+ """,
+ PSERV_FAULT.COBBLER_AUTH_FAILED: """
+ Failed to authenticate with the Cobbler server.
+ """,
+ PSERV_FAULT.COBBLER_AUTH_ERROR: """
+ Failed to authenticate with the Cobbler server.
+ """,
+ PSERV_FAULT.NO_SUCH_PROFILE: """
+ Missing system profile: %(fault_string)s.
+ """,
+ PSERV_FAULT.GENERIC_COBBLER_ERROR: """
+ Unknown problem encountered with the Cobbler server.
+ """,
+ 8002: """
+ Unable to reach provisioning server.
+ """,
+}
+
+
+def _present_user_friendly_fault(fault, presentations):
"""Return a more user-friendly exception to represent `fault`.
:param fault: An exception raised by, or received across, xmlrpc.
:type fault: :class:`xmlrpclib.Fault`
+ :param presentations: A mapping error -> message.
+ :type fault: dict
:return: A more user-friendly exception, if one can be produced.
Otherwise, this returns None and the original exception should be
re-raised. (This is left to the caller in order to minimize
@@ -94,7 +122,7 @@
'fault_code': fault.faultCode,
'fault_string': fault.faultString,
}
- user_friendly_text = PRESENTATIONS.get(fault.faultCode)
+ user_friendly_text = presentations.get(fault.faultCode)
if user_friendly_text is None:
return None
else:
@@ -102,6 +130,34 @@
user_friendly_text.lstrip('\n') % params))
+present_user_friendly_fault = partial(
+ _present_user_friendly_fault, presentations=SHORT_PRESENTATIONS)
+"""Return a concise but user-friendly exception to represent `fault`.
+
+:param fault: An exception raised by, or received across, xmlrpc.
+:type fault: :class:`xmlrpclib.Fault`
+:return: A more user-friendly exception, if one can be produced.
+ 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:`MAASAPIException`, or None.
+"""
+
+
+present_detailed_user_friendly_fault = partial(
+ _present_user_friendly_fault, presentations=DETAILED_PRESENTATIONS)
+"""Return a detailed and user-friendly exception to represent `fault`.
+
+:param fault: An exception raised by, or received across, xmlrpc.
+:type fault: :class:`xmlrpclib.Fault`
+:return: A more user-friendly exception, if one can be produced.
+ 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:`MAASAPIException`, or None.
+"""
+
+
class ProvisioningCaller:
"""Wrapper for an XMLRPC call.
=== modified file 'src/maasserver/tests/test_provisioning.py'
--- src/maasserver/tests/test_provisioning.py 2012-04-10 11:19:49 +0000
+++ src/maasserver/tests/test_provisioning.py 2012-04-11 08:28:18 +0000
@@ -30,11 +30,13 @@
compose_cloud_init_preseed,
compose_commissioning_preseed,
compose_preseed,
+ DETAILED_PRESENTATIONS,
get_metadata_server_url,
name_arch_in_cobbler_style,
+ present_detailed_user_friendly_fault,
present_user_friendly_fault,
- PRESENTATIONS,
select_profile_for_node,
+ SHORT_PRESENTATIONS,
)
from maasserver.testing.enum import map_enum
from maasserver.testing.factory import factory
@@ -113,6 +115,66 @@
self.assertEqual(token.key, maas_dict['token_key'])
self.assertEqual(token.secret, maas_dict['token_secret'])
+ def test_present_detailed_user_friendly_fault_describes_pserv_fault(self):
+ self.assertIn(
+ "provisioning server",
+ present_user_friendly_fault(Fault(8002, 'error')).message)
+
+ def test_present_detailed_fault_covers_all_pserv_faults(self):
+ all_pserv_faults = set(map_enum(PSERV_FAULT).values())
+ presentable_pserv_faults = set(DETAILED_PRESENTATIONS.keys())
+ self.assertItemsEqual([], all_pserv_faults - presentable_pserv_faults)
+
+ def test_present_detailed_fault_rerepresents_all_pserv_faults(self):
+ fault_string = factory.getRandomString()
+ for fault_code in map_enum(PSERV_FAULT).values():
+ original_fault = Fault(fault_code, fault_string)
+ new_fault = present_detailed_user_friendly_fault(original_fault)
+ self.assertNotEqual(fault_string, new_fault.message)
+
+ def test_present_detailed_fault_describes_cobbler_fault(self):
+ friendly_fault = present_detailed_user_friendly_fault(
+ Fault(PSERV_FAULT.NO_COBBLER, factory.getRandomString()))
+ friendly_text = friendly_fault.message
+ self.assertIn("unable to reach", friendly_text)
+ self.assertIn("Cobbler", friendly_text)
+
+ def test_present_detailed_fault_describes_cobbler_auth_fail(self):
+ friendly_fault = present_detailed_user_friendly_fault(
+ Fault(PSERV_FAULT.COBBLER_AUTH_FAILED, factory.getRandomString()))
+ friendly_text = friendly_fault.message
+ self.assertIn("failed to authenticate", friendly_text)
+ self.assertIn("Cobbler", friendly_text)
+
+ def test_present_detailed_fault_describes_cobbler_auth_error(self):
+ friendly_fault = present_detailed_user_friendly_fault(
+ Fault(PSERV_FAULT.COBBLER_AUTH_ERROR, factory.getRandomString()))
+ friendly_text = friendly_fault.message
+ self.assertIn("authentication token", friendly_text)
+ self.assertIn("Cobbler", friendly_text)
+
+ def test_present_detailed_fault_describes_missing_profile(self):
+ profile = factory.getRandomString()
+ friendly_fault = present_detailed_user_friendly_fault(
+ Fault(
+ PSERV_FAULT.NO_SUCH_PROFILE,
+ "invalid profile name: %s" % profile))
+ friendly_text = friendly_fault.message
+ self.assertIn(profile, friendly_text)
+ self.assertIn("maas-import-isos", friendly_text)
+
+ def test_present_detailed_fault_describes_generic_cobbler_fail(self):
+ error_text = factory.getRandomString()
+ friendly_fault = present_detailed_user_friendly_fault(
+ Fault(PSERV_FAULT.GENERIC_COBBLER_ERROR, error_text))
+ friendly_text = friendly_fault.message
+ self.assertIn("Cobbler", friendly_text)
+ self.assertIn(error_text, friendly_text)
+
+ def test_present_detailed_fault_returns_None_for_other_fault(self):
+ self.assertIsNone(
+ present_detailed_user_friendly_fault(Fault(9999, "!!!")))
+
def test_present_user_friendly_fault_describes_pserv_fault(self):
self.assertIn(
"provisioning server",
@@ -120,7 +182,7 @@
def test_present_user_friendly_fault_covers_all_pserv_faults(self):
all_pserv_faults = set(map_enum(PSERV_FAULT).values())
- presentable_pserv_faults = set(PRESENTATIONS.keys())
+ presentable_pserv_faults = set(SHORT_PRESENTATIONS.keys())
self.assertItemsEqual([], all_pserv_faults - presentable_pserv_faults)
def test_present_user_friendly_fault_rerepresents_all_pserv_faults(self):
@@ -134,22 +196,21 @@
friendly_fault = present_user_friendly_fault(
Fault(PSERV_FAULT.NO_COBBLER, factory.getRandomString()))
friendly_text = friendly_fault.message
- self.assertIn("unable to reach", friendly_text)
- self.assertIn("Cobbler", friendly_text)
+ self.assertIn("Unable to reach the Cobbler server", 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.message
- self.assertIn("failed to authenticate", friendly_text)
- self.assertIn("Cobbler", friendly_text)
+ self.assertIn(
+ "Failed to authenticate with the Cobbler server", 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.message
- self.assertIn("authentication token", friendly_text)
- self.assertIn("Cobbler", friendly_text)
+ self.assertIn(
+ "Failed to authenticate with the Cobbler server", friendly_text)
def test_present_user_friendly_fault_describes_missing_profile(self):
profile = factory.getRandomString()
@@ -159,18 +220,15 @@
"invalid profile name: %s" % profile))
friendly_text = friendly_fault.message
self.assertIn(profile, friendly_text)
- self.assertIn("maas-import-isos", friendly_text)
def test_present_user_friendly_fault_describes_generic_cobbler_fail(self):
error_text = factory.getRandomString()
friendly_fault = present_user_friendly_fault(
Fault(PSERV_FAULT.GENERIC_COBBLER_ERROR, error_text))
friendly_text = friendly_fault.message
- self.assertIn("Cobbler", friendly_text)
- self.assertIn(error_text, friendly_text)
-
- def test_present_user_friendly_fault_returns_None_for_other_fault(self):
- self.assertIsNone(present_user_friendly_fault(Fault(9999, "!!!")))
+ self.assertIn(
+ "Unknown problem encountered with the Cobbler server.",
+ friendly_text)
class ProvisioningTests:
