https://fedorahosted.org/sssd/ticket/1110
[PATCH 1/4] I've removed FIXME and move the DEBUG message to a better place.
I would have Acked that, but I agree with Jakub that this should be done as a part of fix for #1125
[PATCH 2/4] I've wrapped the data provider part with tevent request so I can reuse the code in periodical updates.
Nack, sdap_sudo.c:144 - please use sdap_handler_done() for better clarity
I'm not quuite sure why are you swicthing to EAGAIN in sdap_sudo_load_sudoers_next_base(). I'd understand if you used EOK as well, but this way I doubt it has any advantage (doesn't even add lucidity IMO).
[PATCH 3/4] Adds two new functions in sudo sysdb api that can set/retrieve the value of 'refreshed' attribute (equivalent to 'has_enumerated' attribute in domain enumeration). If it is true, the first schedule will be delayed for a few seconds so we don't slow down the start up process.
Nack,
I think that re-using sysdb_set_enumerated() and sysdb_has_enumarated() and writing wrappers around them is far better than copying the code, which is almost identical.
[PATCH 4/4] Periodical update itself. It adds two new options: ldap_sudo_refresh_enabled (bool) - equivalent to 'enumerate' ldap_sudo_refresh_timeout (number) - equivalent to 'ldap_enumeration_refresh_timeout'
Nack, Add new options to config description in src/config/, also please review if all of your original SUDO-related options are there as well
I think that delaying the refresh for ldap_sudo_refresh_timeout should be used instead of hardcoded 10 seconds. Or do you think that default value for the option is too high for this?
Nitpick: in the definition of sdap_sudo_refresh_timer(), you miss the "static" keyword
You are forgetting to free sudo_ctx in sdap_sudo_refresh_timer, which could be potentially deadly on long-running systems, since the refresh_ctx isn't freed anywhere as weel
Please use the standard _send/_retry/_done/_recv function naming in sdap_sudo_timer.c
In sdap_sudo_refresh_send, you create request with struct sdap_sudo_refresh_state as its state, but in the timeout handling procedure, you retrieve callback data of that request as struct sdap_sudo_refresh_ctx. Please correct that and keep in mind that since this is entirely different struct assigned to the request, freeing the request won't free the sudo_ctx created in sdap_sudo_refresh_timer().
Thanks Jan