Sorry for breaking the thread, but my email server seems to have lost track of the original emails (they're not showing up in my folders).
Jakub, thanks for catching the ioa_common.c issue. Fixed.
I didn't bother differentiating the messages in the API because they're A) unimportant and B) not sufficiently different to be worth adding new translatable strings for.
I did fix the manpages though. Those SHOULD be documented.
New patch attached.
Also, this is designed to apply atop my patches for the RootDSE/search base fix (which still needs reviewing). See "[PATCH] LDAP: Do not fail if RootDSE check cannot determine search bases"
On Fri, Feb 03, 2012 at 10:31:10AM -0500, Stephen Gallagher wrote:
Sorry for breaking the thread, but my email server seems to have lost track of the original emails (they're not showing up in my folders).
Jakub, thanks for catching the ioa_common.c issue. Fixed.
I didn't bother differentiating the messages in the API because they're A) unimportant and B) not sufficiently different to be worth adding new translatable strings for.
I did fix the manpages though. Those SHOULD be documented.
New patch attached.
Also, this is designed to apply atop my patches for the RootDSE/search base fix (which still needs reviewing). See "[PATCH] LDAP: Do not fail if RootDSE check cannot determine search bases"
This patch works for me. I don't have a strong opinion on passing the sss_domain_ctx structure versus retreiving it with sysdb_ctx_get_domain (See Jan's reply to the original thread).
So ack from me, feel free to resend or not.
On Fri, 2012-02-03 at 10:31 -0500, Stephen Gallagher wrote:
Sorry for breaking the thread, but my email server seems to have lost track of the original emails (they're not showing up in my folders).
Jakub, thanks for catching the ioa_common.c issue. Fixed.
I didn't bother differentiating the messages in the API because they're A) unimportant and B) not sufficiently different to be worth adding new translatable strings for.
I did fix the manpages though. Those SHOULD be documented.
New patch attached.
Also, this is designed to apply atop my patches for the RootDSE/search base fix (which still needs reviewing). See "[PATCH] LDAP: Do not fail if RootDSE check cannot determine search bases"
This is a matter of taste I guess but I find it difficult to read and remember th anme you cage to the options. Why entry_cache_user_timeout and not just user_cache_timeout (and so on for other maps) ? It sounds more readable to me.
Also I understand why you do these changes: - timeout = dp_opt_get_int(state->opts->basic, SDAP_ENTRY_CACHE_TIMEOUT); + timeout = dp_opt_get_int(state->opts->basic, SDAP_SEARCH_TIMEOUT);
But I think they belong in a separate patch, as they seem to fix using the wrong timeout but are not directly related to the change the patch is about ?
Everything else looks fine.
Simo.
On Fri, 2012-02-03 at 14:32 -0500, Simo Sorce wrote:
On Fri, 2012-02-03 at 10:31 -0500, Stephen Gallagher wrote:
Sorry for breaking the thread, but my email server seems to have lost track of the original emails (they're not showing up in my folders).
Jakub, thanks for catching the ioa_common.c issue. Fixed.
I didn't bother differentiating the messages in the API because they're A) unimportant and B) not sufficiently different to be worth adding new translatable strings for.
I did fix the manpages though. Those SHOULD be documented.
New patch attached.
Also, this is designed to apply atop my patches for the RootDSE/search base fix (which still needs reviewing). See "[PATCH] LDAP: Do not fail if RootDSE check cannot determine search bases"
This is a matter of taste I guess but I find it difficult to read and remember th anme you cage to the options. Why entry_cache_user_timeout and not just user_cache_timeout (and so on for other maps) ? It sounds more readable to me.
Also I understand why you do these changes:
- timeout = dp_opt_get_int(state->opts->basic, SDAP_ENTRY_CACHE_TIMEOUT);
- timeout = dp_opt_get_int(state->opts->basic, SDAP_SEARCH_TIMEOUT);
But I think they belong in a separate patch, as they seem to fix using the wrong timeout but are not directly related to the change the patch is about ?
Everything else looks fine.
Sure, I agree. When I push them I'll split them into a separate patch.
On Fri, 2012-02-03 at 15:20 -0500, Stephen Gallagher wrote:
On Fri, 2012-02-03 at 14:32 -0500, Simo Sorce wrote:
On Fri, 2012-02-03 at 10:31 -0500, Stephen Gallagher wrote:
Sorry for breaking the thread, but my email server seems to have lost track of the original emails (they're not showing up in my folders).
Jakub, thanks for catching the ioa_common.c issue. Fixed.
I didn't bother differentiating the messages in the API because they're A) unimportant and B) not sufficiently different to be worth adding new translatable strings for.
I did fix the manpages though. Those SHOULD be documented.
New patch attached.
Also, this is designed to apply atop my patches for the RootDSE/search base fix (which still needs reviewing). See "[PATCH] LDAP: Do not fail if RootDSE check cannot determine search bases"
This is a matter of taste I guess but I find it difficult to read and remember th anme you cage to the options. Why entry_cache_user_timeout and not just user_cache_timeout (and so on for other maps) ? It sounds more readable to me.
Also I understand why you do these changes:
- timeout = dp_opt_get_int(state->opts->basic, SDAP_ENTRY_CACHE_TIMEOUT);
- timeout = dp_opt_get_int(state->opts->basic, SDAP_SEARCH_TIMEOUT);
But I think they belong in a separate patch, as they seem to fix using the wrong timeout but are not directly related to the change the patch is about ?
Everything else looks fine.
Sure, I agree. When I push them I'll split them into a separate patch.
For posterity (and to make it easier on me to keep track of what needs pushing), here are the two new patches.
I left the option names alone, for the record. I prefer the shorter names, and it's a purely cosmetic decision.
On Fri, 2012-02-03 at 15:35 -0500, Stephen Gallagher wrote:
On Fri, 2012-02-03 at 15:20 -0500, Stephen Gallagher wrote:
On Fri, 2012-02-03 at 14:32 -0500, Simo Sorce wrote:
On Fri, 2012-02-03 at 10:31 -0500, Stephen Gallagher wrote:
Sorry for breaking the thread, but my email server seems to have lost track of the original emails (they're not showing up in my folders).
Jakub, thanks for catching the ioa_common.c issue. Fixed.
I didn't bother differentiating the messages in the API because they're A) unimportant and B) not sufficiently different to be worth adding new translatable strings for.
I did fix the manpages though. Those SHOULD be documented.
New patch attached.
Also, this is designed to apply atop my patches for the RootDSE/search base fix (which still needs reviewing). See "[PATCH] LDAP: Do not fail if RootDSE check cannot determine search bases"
This is a matter of taste I guess but I find it difficult to read and remember th anme you cage to the options. Why entry_cache_user_timeout and not just user_cache_timeout (and so on for other maps) ? It sounds more readable to me.
Also I understand why you do these changes:
- timeout = dp_opt_get_int(state->opts->basic, SDAP_ENTRY_CACHE_TIMEOUT);
- timeout = dp_opt_get_int(state->opts->basic, SDAP_SEARCH_TIMEOUT);
But I think they belong in a separate patch, as they seem to fix using the wrong timeout but are not directly related to the change the patch is about ?
Everything else looks fine.
Sure, I agree. When I push them I'll split them into a separate patch.
For posterity (and to make it easier on me to keep track of what needs pushing), here are the two new patches.
I left the option names alone, for the record. I prefer the shorter names, and it's a purely cosmetic decision.
Pushed to master.
sssd-devel@lists.fedorahosted.org