[Freeipa-devel] [PATCH 0274] DNS: Check if dns package is installed

[Tomas Babej]

On 07/02/2015 02:03 PM, Petr Spacek wrote:
> On 2.7.2015 13:54, Jan Cholasta wrote:
>> Dne 2.7.2015 v 13:34 Petr Spacek napsal(a):
>>> On 2.7.2015 12:57, Tomas Babej wrote:
>>>>
>>>>
>>>> On 07/02/2015 08:50 AM, Petr Spacek wrote:
>>>>> On 1.7.2015 20:29, Tomas Babej wrote:
>>>>>>
>>>>>>
>>>>>> On 07/01/2015 04:45 PM, Petr Spacek wrote:
>>>>>>> On 1.7.2015 15:32, Martin Basti wrote:
>>>>>>>> https://fedorahosted.org/freeipa/ticket/4058
>>>>>>>> Requires patch freeipa-pspacek-0052
>>>>>>>
>>>>>>> ACK
>>>>>>>
>>>>>>
>>>>>> I must admit I don't really like wrapping a constant in the method in
>>>>>> the TaskNamespace object.
>>>>>>
>>>>>> We're interested in the constant itself - there's no case I can imagine
>>>>>> where the name of the freeipa's dns package will be dynamic.
>>>>>>
>>>>>> For paths we have BasePathNamespace that contains all the paths, maybe
>>>>>> we should introduce something similar for the non-path platform
>>>>>> dependent constants?
>>>>>
>>>>> Generally I support this but it seems like a 4.3 material (and out of
>>>>> scope of
>>>>> #4058). We need to finish 4.2 now.
>>>>>
>>>>> Please ACK or NACK ASAP.
>>>>>
>>>>
>>>> It's fairly straightforward to introduce a new platform namespace for
>>>> constants.
>>>>
>>>> See attached patch, it implements the namespace and already contains the
>>>> proper values for the dns package name.
>>>>
>>>> The original patch 274 would only need to use:
>>>>
>>>>      >>> from ipaplatform.constants import constants
>>>>      >>> constants.DNS_PACKAGE_NAME
>>>>      'freeipa-server-dns'
>>>
>>> I'm okay with that if Honza or somebody else knowledgable about the whole
>>> platform-thingy can ACK this, amend Martin^2's patch 274 and test the whole
>>> thing.
>>>
>>> Unfortunately I do not have time for it myself. If nobody does that please
>>> push the original patch (when it's dependency pspacek-0052 gets ACK).
>>>
>>
>> I think you are overengineering this a little bit, adding whatever ipaplatform
>> stuff just because of an error message seems rather unnecessary to me. I think
>> changing the error message to "Integrated DNS requires 'freeipa-server-dns'
>> package" or even "Integrated DNS requires IPA DNS server package" would be
>> perfectly fine.
> 
> The message should be as specific as possible but I do not care how it will be
> implemented.
> 

Alright, let's not get stuck. Petr insists on specific message on each
platform. Given that package name is platform dependent, I think we
should keep it as platform constant, task makes little sense.

Given that Martin's not available right now, I'll amend his patches and
send the updated version.

Tomas