[Freeipa-devel] [PATCH] Removed duplicate domain name validation function

[Martin Basti]

On 01.12.2015 16:03, Stanislav Laznicka wrote:
> Sending the patch with renamed function.
>
> Standa
>
> On 12/01/2015 09:57 AM, Jan Cholasta wrote:
>> On 1.12.2015 09:37, Petr Spacek wrote:
>>> On 30.11.2015 20:00, Martin Basti wrote:
>>>>
>>>>
>>>> On 27.11.2015 16:06, Stanislav Laznicka wrote:
>>>>> Please, see the modified patch attached.
>>>>>
>>>>> Standa
>>>>>
>>>>> On 11/27/2015 03:48 PM, Martin Basti wrote:
>>>>>>
>>>>>>
>>>>>> On 27.11.2015 15:33, Petr Spacek wrote:
>>>>>>> On 27.11.2015 15:32, Martin Basti wrote:
>>>>>>>>
>>>>>>>> On 25.11.2015 17:18, Stanislav Laznicka wrote:
>>>>>>>>> There were two functions for the same purpose. Removed one.
>>>>>>>>>
>>>>>>>>>
>>>>>>>> Hello,
>>>>>>>>
>>>>>>>> I would like to have "log" param of is_host_resolvable as optional
>>>>>>> Is there an immediate need for the optional param? If not, I 
>>>>>>> would not
>>>>>>> clutter
>>>>>>> the code.
>>>>>>>
>>>>>> So at least I would like to move log param as the last param, in 
>>>>>> case of
>>>>>> need it can be modified to optional parameter.
>>>>>>
>>>>>> Or log can be default as root_logger, IMO we us only root_logger
>>>>>> everywhere, but this need investigation.
>>>>>>
>>>>>> Martin
>>>>>
>>>> It works, but I would like to have Honza's opinion if 
>>>> ipalib/util.py is the
>>>> right place for the new method and if we really need to use 
>>>> exceptions.
>>
>> ipalib/util.py is fine for the function, since it is not used 
>> anywhere in ipapython.
>>
>> I would rename the function to verify_host_resolvable() though, since 
>> the is_ prefix suggests the function returns a boolean value rather 
>> than raises an exception or not.
>>
>>>>
>>>> IMO is_record_resolvable() should return only True/False and then 
>>>> it can be
>>>> located in ipapython module
>>>
>>> Hmm, I do not see a reason why it should not throw an exception. 
>>> IMHO the
>>> exception should contain the original error/result from resolver, so 
>>> it is
>>> possible to print meaningful error message like "DNS query timed 
>>> out" instead
>>> of "it does not work for some reason, trust us".
>>>
>>
>>
>
Pushed to master: 498471e4aed1367b72cd74d15811d0584a6ee268

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20151202/3ef73e59/attachment.htm>