[Freeipa-devel] [PATCH] 0082/0083 Handle NotFound exception when establishing trust
Alexander Bokovoy
abokovoy at redhat.com
Fri Oct 5 18:14:52 UTC 2012
On Fri, 05 Oct 2012, Petr Vobornik wrote:
>On 10/05/2012 03:24 PM, Alexander Bokovoy wrote:
>>On Fri, 05 Oct 2012, Petr Vobornik wrote:
>>>On 10/04/2012 05:06 PM, Alexander Bokovoy wrote:
>>>>
>>>>Hi,
>>>>
>>>>two attached patches attempt to solve
>>>>https://fedorahosted.org/freeipa/ticket/3103
>>>>
>>>>We cannot make educated guess where trusted domain's DNS server is
>>>>located as we ended up with NotFound exception precisely because we were
>>>>unable to discover trusted domain's domain controller location.
>>>>
>>>>Thus, all we can do is to suggest ways to fix the issue. Since
>>>>suggestion becomes relatively long (multi-line, at least), it creates
>>>>few issues for current exception error message handling:
>>>> - root_logger does not use textui to format output
>>>> - textui is only printing to standard output
>>>> - multi-line error messages thus become non-wrapped
>>>> - localizing such messages would become a harder context-wise.
>>>>
>>>>Web UI is showing error message as a single paragraph (<p/>), all
>>>>multi-line markup would be lost.
>>>>
>>>>In the end, I decided to support list-based multi-line error messages in
>>>>PublicError class. Its constructor will join all list-based arguments
>>>>into single string per argument (first patch).
>>>>
>>>>In Web UI I've added special text processing of error messages that
>>>>splits message into multiple lines and wraps those which start with a
>>>>TAB symbol into 'error-message-hinted' span to visually separate it from
>>>>the rest of error message. Trust code uses this to display suggested CLI
>>>>command to set up DNS forwarder (second patch).
>>>>
>>>>In the end, CLI shows such error message fine (note tabulated CLI
>>>>command):
>>>>-----------------------------------------------------------------------
>>>># ipa trust-add --type=ad --admin Administrator at ad.local1 --password
>>>>ad.local1
>>>>Active directory domain administrator's password: ipa: ERROR: Unable to
>>>>resolve domain controller for 'ad.local1' domain. IPA manages DNS,
>>>>please configure forwarder to 'ad.local1' domain by using following CLI
>>>>command. Please replace DNS_SERVER and IP_ADDRESS by name and address of
>>>>the domain's name server:
>>>> ipa dnszone-add ad.local1 --name-server=DNS_SERVER
>>>>--admin-email='hostmaster at ad.local1' --force --forwarder=IP_ADDRESS
>>>>--forward-policy=only
>>>>When using Web UI, please create DNS zone for domain 'ad.local1' first
>>>>and then set forwarder and forward policy
>>>>-----------------------------------------------------------------------
>>>>
>>>>Web UI looks like this: http://abbra.fedorapeople.org/.paste/ui.png
>>>>
>>>>
>>>
>>>You have undeclared identifier: lines.
>>>
>>>I recommend to run `jsl -conf jsl.conf` command in install/ui folder
>>>to prevent these issues.
>>Fixed.
>>
>>
>>>I'm not convinced that "beautify" call should be in command object. I
>>>would rather see it in error_dialog.
>>I moved everything to error_dialog and used $() selectors instead of
>>directly playing with text.
>
>1)
>+ var error_message = $('<span />', {});
>
>I would rather see a <div /> as a container. Span is and should
>contain only inline elements.
Fixed.
>2)
>var line_span = $('<span />', {
> class: 'error-message-hinted',
> text: lines[i].substr(1)
> }).appendTo(container);
>
>Why do you use <span> for hinted message and <p> for other lines?
>Shouldn't we use <p> for both?
Fixed to use <p> everywhere.
>3) var line_span is declared twice. JS use function scope, not block scope.
Fixed.
Thanks for the review. New patch 0082 attached.
--
/ Alexander Bokovoy
-------------- next part --------------
>From 5a85259e688aae66d740ba6407e60a0fcc601964 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <abokovoy at redhat.com>
Date: Thu, 4 Oct 2012 15:05:17 +0300
Subject: [PATCH 01/12] support multi-line error messages in exceptions
---
install/ui/ipa.css | 9 ++++++++-
install/ui/ipa.js | 31 +++++++++++++++++++++++++------
ipalib/errors.py | 12 +++++++++---
3 files changed, 42 insertions(+), 10 deletions(-)
diff --git a/install/ui/ipa.css b/install/ui/ipa.css
index bc971dce4aa3377fc9918ed3c16a89565f505c94..4e51c3051e75846f386910c8998f73db7afbddaa 100644
--- a/install/ui/ipa.css
+++ b/install/ui/ipa.css
@@ -1112,6 +1112,13 @@ table.kerberos-key-status {
background-color: #daa520;
}
+.error-message-hinted {
+ color: red;
+ padding-top: 0.5em;
+ padding-bottom: 0.5em;
+ font-family: monospace;
+}
+
/* ---- Table ---- */
table.scrollable thead {
@@ -1784,4 +1791,4 @@ form#login {
.choice-header {
font-weight: bold;
-}
\ No newline at end of file
+}
diff --git a/install/ui/ipa.js b/install/ui/ipa.js
index 45195bc499f874426020cd400a1ae9a5c1ef5f0f..8f924b9d29262282f3ad771653e64892c2945efc 100644
--- a/install/ui/ipa.js
+++ b/install/ui/ipa.js
@@ -1419,6 +1419,25 @@ IPA.error_dialog = function(spec) {
that.visible_buttons = spec.visible_buttons || ['retry', 'cancel'];
};
+ that.beautify_message = function(container, message) {
+ var lines = message.split(/\n/g);
+ var line_span;
+ for(var i=0; i<lines.length; i++) {
+ // multi-lined text may contain TAB character as first char of the line
+ // to hint at marking the whole line differently
+ if (lines[i].charAt(0) == '\t') {
+ line_span = $('<p />', {
+ class: 'error-message-hinted',
+ text: lines[i].substr(1)
+ }).appendTo(container);
+ } else {
+ line_span = $('<p />', {
+ text: lines[i]
+ }).appendTo(container);
+ }
+ }
+ };
+
that.create = function() {
if (that.error_thrown.url) {
$('<p/>', {
@@ -1426,9 +1445,9 @@ IPA.error_dialog = function(spec) {
}).appendTo(that.container);
}
- $('<p/>', {
- html: that.error_thrown.message
- }).appendTo(that.container);
+ var error_message = $('<div />', {});
+ that.beautify_message(error_message, that.error_thrown.message);
+ error_message.appendTo(that.container);
if(that.errors && that.errors.length > 0) {
//render errors
@@ -1457,9 +1476,9 @@ IPA.error_dialog = function(spec) {
for(var i=0; i < that.errors.length; i++) {
var error = that.errors[i];
if(error.message) {
- var error_div = $('<li />', {
- text: error.message
- }).appendTo(errors_container);
+ var error_div = $('<li />', {});
+ that.beautify_message(error_div, error.message);
+ error_div.appendTo(errors_container);
}
}
diff --git a/ipalib/errors.py b/ipalib/errors.py
index 7bf267290b484ed9570f1c7efdb606628fb5f11d..7f1113200b6b2cd80ea9156d347211e4d978b9be 100644
--- a/ipalib/errors.py
+++ b/ipalib/errors.py
@@ -265,11 +265,17 @@ class PublicError(StandardError):
)
self.format = format
self.forwarded = False
- self.msg = self.format % kw
+ def convert_keyword(value):
+ if isinstance(value, list):
+ result=u'\n'.join(map(lambda line: unicode(line), value))
+ return result
+ return value
+ kwargs = dict(map(lambda (k,v): (k, convert_keyword(v)), kw.items()))
+ self.msg = self.format % kwargs
if isinstance(self.format, basestring):
- self.strerror = ugettext(self.format) % kw
+ self.strerror = ugettext(self.format) % kwargs
else:
- self.strerror = self.format % kw
+ self.strerror = self.format % kwargs
else:
if isinstance(message, (Gettext, NGettext)):
message = unicode(message)
--
1.7.12
More information about the Freeipa-devel
mailing list