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

[Jan Cholasta]

Dne 20.7.2015 v 19:00 Martin Babinsky napsal(a):
> On 07/17/2015 02:37 PM, Martin Basti wrote:
>> On 03/07/15 09:03, Tomas Babej wrote:
>>>
>>> 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
>> Updated patches attached.
>>
>> ACK for 332
>> I just removed DNS constants from 332 patch
>>
>>
>>
> ACK for Martin's patch.

Pushed to:
master: 92828d3cf50e00fe75ebf3ec9e0edc8b9c8eae35
ipa-4-2: eefe6dc3a2a6fb316650638c3db90ed63f8551de

-- 
Jan Cholasta