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

[Martin Basti]

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

-- 
Martin Basti

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbasti-0274-2-DNS-check-if-DNS-package-is-installed.patch
Type: text/x-patch
Size: 7153 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150717/8914d6f5/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0332-2-ipaplatform-Add-constants-submodule.patch
Type: text/x-patch
Size: 4971 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150717/8914d6f5/attachment-0001.bin>