[Freeipa-devel] [PATCH 0473-0476, 0478-0482]DNS Locations: Prologue

[Martin Kosek]

On 06/03/2016 12:51 PM, Martin Basti wrote:
> 
> 
> On 03.06.2016 08:53, Petr Spacek wrote:
>> On 2.6.2016 17:53, Martin Basti wrote:
>>> <snip>
>>>>>>>> Typo - redundant ' ' at the end.
>>>>>>>>
>>>>>>>>
>>>>>>>> Conditional NACK, warnings mentioned in
>>>>>>>> http://www.freeipa.org/page/V4/DNS_Location_Mechanism#CLI
>>>>>>>> are not there.
>>>>>>>>
>>>>>>>> I'm open to changing this to ACK if you open a separate ticket for this
>>>>>>>> omission so we do not forget to add them later on.
>>> I forgot to add, this will be in next batch of patches (you may see that there
>>> are not marked DNS servers in output of location show), I do not see reason to
>>> open ticket when the current one is not finished.
>>>
>>>>>>> +1
>>>>>>>
>>>> Done
>>>>
>>>>>>> Patch 480:
>>>>>>>
>>>>>>> 1) The code in location_show.execute() looks like it could be moved to
>>>>>>> location_show.post_callback()
>>>>>>>
>>>> I had to add it to execute because I modifies result entry not just
>>>> entry_attrs
>>>>
>>>>>>> 2) Before calling super().output_for_cli(), pop 'servers' from result, so
>>>>>>> that
>>>>>>> it is not displayed with --all.
>>>>>>>
>>>>>>>
>>>> Done
>>>>
>>>>>>> Patch 481:
>>>>>>>
>>>>>>> 1) Could we rename --force to --nonempty (or something better)? I would
>>>>>>> like
>>>>>>> to reserve --force for "ignore NotFound when deleting the entry", which
>>>>>>> is not
>>>>>>> the case here.
>>>>>> IMHO option is unnecessary. Just delete the location (and unset location
>>>>>> from
>>>>>> all member servers). The design does not contain --force anyway :-)
>>>>> OK, that's even better :-)
>>>>>
>>>> Done
>>>>
>>>> Updated patches attached
>> I had to add top object class to the plugin and tests to make tests pass.
>> Patch is attached.
>>
>> CondACK: Fix this before pushing somehow.
>>
> 
> Updated and heavily rebased patches attached.

Hi guys,

I saw the patches were merged, great! I just noticed that you added referential
integrity for dnslocation attribute. In that case, you will want to also add
QE, PRES index for it, to keep performance on reasonable level. Or was the
omission of index already discussed and justified?

Thanks,
Martin