[Freeipa-devel] [PATCH 0017] Improve error message in ipa-replica-manage

[Tomas Babej]

On 10/19/2012 09:55 AM, Petr Viktorin wrote:
> On 10/18/2012 08:01 PM, Rob Crittenden wrote:
>> Tomas Babej wrote:
>>> On 10/02/2012 03:55 PM, Rob Crittenden wrote:
>>>> Tomas Babej wrote:
>>>>> Hi,
>>>>>
>>>>> When executing ipa-replica-manage connect to an unknown or irrelevant
>>>>> master, we now print a sensible error message informing the user
>>>>> about this possiblity as well.
>>>>>
>>>>> https://fedorahosted.org/freeipa/ticket/3105
>>>>>
>>>>> Tomas
>>>>
>>>> I put a whole bunch of code into a try/except and this may be catching
>>>> errors in unexpected ways.
>>>>
>>>> I'm not entirely sure right now what we should do, but looking at the
>>>> code in the try:
>>>>
>>>> repl1.conn.getEntry(master1_dn, ldap.SCOPE_BASE)
>>>> repl1.conn.getEntry(master2_dn, ldap.SCOPE_BASE)
>>>>
>>>> We take in replica1 and replica1 as arguments (the default for
>>>> replica1 is the current host).
>>>>
>>>> If either of these raise a NotFound it means there there is no master
>>>> of that name. Does that mean that the master was deleted? Well,
>>>> clearly not.
>>>>
>>>> A lot has changed since I did this, I may have been relying on a
>>>> side-effect, or just hadn't tested well-enough.
>>>>
>>>> I wonder if we need that message at all. Is "foo" is not an IPA server
>>>> good enough? It still might be confusing if someone didn't know that
>>>> "foo" was deleted and it was still running. We could probably verify
>>>> that it is at least an IPA server by doing similar checking in the
>>>> client, it all depends on how far we want to take it.
>>>>
>>>> rob
>>>
>>> I modified the patch. Now if the NotFound error is encountered, we try
>>> to investigate whether we're trying to connect to an IPA server at all.
>>> Please see if you have any suggestions.
>>>
>>> Tomas
>>
>> Getting there, the errors are a lot better.
>>
>> Can you modify the 'Connection unsuccessful' message to include the
>> server we failed to connect to?
>>
>> The logger isn't so nice either, I think I'd prefer it if we could
>> exclude the severity:
>>
>> ipa: ERROR: LDAP Error: Connect error: TLS error -8172:Peer's
>> certificate issuer has been marked as not trusted by the user.
>> Connection unsuccessful.
>>
>> So drop the 'ipa: ERROR: ' part for consistency. TI don't believe we
>> configure the root logger at all in this tool so it is using the 
>> defaults.
>
> It's not very easy to find the right call to configure the logger to 
> drop the "ipa: ERROR:" part:
> standard_logging_setup(console_format='%(message)s')
> The function is in ipapython.ipa_log_manager. Hopefully that helps.
>
Thanks!
>> I'd also replace the test for -4 with the constant
>> ipadiscovery.NOT_IPA_SERVER
>>
>> rob
>>
>
I implemented your suggestions. Please have a look at the new patch version.

Tomas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0017-3-IPA-Server-check-in-ipa-replica-manage.patch
Type: text/x-patch
Size: 2224 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20121022/ad4ec00d/attachment.bin>