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

[Martin Babinsky]

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.

-- 
Martin^3 Babinsky