This has to be applied atop the first way.
Feel free to ping me for testing environment.
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>
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.
On Wed, Apr 10, 2013 at 02:28:39PM +0200, Pavel Březina wrote:
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.
Ack
sssd-devel@lists.fedorahosted.org