[Freeipa-devel] [PATCH] 0082/0083 Handle NotFound exception when establishing trust

[Petr Vobornik]

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.

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?

3) var line_span is declared twice. JS use function scope, not block scope.

>
>> I'm not sure that all tabbed error text should be red. But I don't
>> recall any other usage so it's probably OK.
> We don't have any, this is first one.
>
>


--
Petr Vobornik