On 04/10/2013 02:13 PM, Jakub Hrozek wrote:
>On Tue, Apr 09, 2013 at 02:36:09PM +0200, Pavel Březina wrote:
>>On 04/08/2013 12:28 PM, Pavel Březina wrote:
>>>This has to be applied atop the first way.
>>>
>>>Feel free to ping me for testing environment.
>>
>>Addressed some review issues raised in other thread.
>>
>
>Maybe you'd change the patch to address the compilation
>issue I raised in the other thread, but the code looks good to me and
>works OK.
>
>I only have concern about the man page and the default.
>
> <varlistentry>
>+ <term>ipa_enable_dns_sites (boolean)</term>
>+ <listitem>
>+ <para>
>+ Enables/disables DNS sites - location based
>+ service discovery.
>
>I think just enables is OK. A boolean can enable and disable by definition :)
>
>+ </para>
>+ <para>
>+ If true and _srv_ is present in ipa_server,
>+ SSSD will search for SRV records in
>+ (1)
_location.hostname.example.com, (2)
example.com.
>+ If (1) exists, it is expected to contain primary
>+ servers and (2) is treated as backup servers. If (1)
>+ does not exist, (2) is treated as primary servers.
>
>Can you run this paragraph by some native speaker? Maybe we could reword it to
>say something like:
>
>"If true and service discovery (see Service Discovery paragraph at the
>bottom of the man page) is enabled, then the SSSD will first attempt
>location based discovery using a query that contains
>"_location.hostname.example.com" and then fall back to traditional SRV
>discovery. If the location based discovery succeeds, the IPA servers located
>with the location based discovery are treated as primary servers and the IPA
>servers located using the traditional SRV discovery are used as back up
>servers".
>
>That was just a suggestion, but the original paragraph was not clear to me.
>
>+ </para>
>+ <para>
>+ Default: true
>
>I think the default should be False until the IPA server supports this
>discovery by defualt, too, otherwise there is a wasted round-trip for every IPA
>client.
>
>+ </para>
>+ </listitem>
>+ </varlistentry>
>
Thanks. Done.