[SSSD] [PATCH] confdb: Make offline timeout configurable

Jakub Hrozek jhrozek at redhat.com
Thu Nov 7 10:39:08 UTC 2013


On Tue, Nov 05, 2013 at 08:14:26AM -0500, Stephen Gallagher wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 11/05/2013 05:48 AM, Jakub Hrozek wrote:
> > On Fri, Nov 01, 2013 at 05:36:54PM +0100, Michal Židek wrote:
> >> On 11/01/2013 01:42 PM, Jakub Hrozek wrote:
> >>> On Fri, Nov 01, 2013 at 07:33:00AM -0400, Stephen Gallagher
> >>> wrote:
> >>>> -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
> >>>> 
> >>>> On 10/31/2013 01:40 PM, Jakub Hrozek wrote:
> >>>>> On Thu, Oct 31, 2013 at 12:54:04PM +0100, Michal Židek
> >>>>> wrote:
> >>>>>> On 10/27/2013 07:32 PM, Jakub Hrozek wrote:
> >>>>>>> On Fri, 2013-10-25 at 20:11 -0400, Simo Sorce wrote:
> >>>>>>>> On Fri, 2013-10-25 at 17:42 +0200, Michal Židek
> >>>>>>>> wrote:
> >>>>>>>>> I decided not to only document the behaviour, but
> >>>>>>>>> also add option to configure it (so I could remove
> >>>>>>>>> one FIXME from the code).
> >>>>>>>>> 
> >>>>>>>>> https://fedorahosted.org/sssd/ticket/1718
> >>>>>>>> 
> >>>>>>>> Thanks a lot, glad to see a long standing FIXME go
> >>>>>>>> away.
> >>>>>>>> 
> >>>>>>>> Ack, Simo.
> >>>>>>>> 
> >>>>>>> 
> >>>>>>> The code itself is good, but you also need to add the
> >>>>>>> parameter to the confAPI. In particular,
> >>>>>>> src/config/etc/sssd.api.conf and then make sure make
> >>>>>>> check passes.
> >>>>>>> 
> >>>>>> 
> >>>>>> Done.
> >>>>>> 
> >>>>>> Michal
> >>>>>> 
> >>>>> 
> >>>>> Sorry I didn't think about it the first time around, but I
> >>>>> think the code should also initialize offline_timeout to
> >>>>> the default value so that if confdb_get_int fails for
> >>>>> whatever reason, we still decide based on the default
> >>>>> value. Ignoring the result if confdb_get_int is fine, but
> >>>>> we could warn with a DEBUG message.
> >>>> 
> >>>> If confdb_get_int() fails, it can pretty much only mean that
> >>>> the value in the config file was not an integer. In that
> >>>> case, we should *really* be quitting.
> >>> 
> >>> Not really, there can be a number of internal errors as well. I
> >>> think the proper solution for the problem you describe is a
> >>> configuration file validator, but that's really out of scope of
> >>> this change.
> >>> 
> >>>> 
> >>>> I'd rather see us check this option at startup and store it
> >>>> in the be_ctx, that way we can abort (or loudly log that it's
> >>>> wrong and fall back to defaults) at startup.
> >>> 
> >>> I don't like extending be_ctx with per-option attributes. What
> >>> if we
> >> 
> >> I was about to create a patch with offline_timeout in be_ctx,
> >> but it did not look very nice to me as well.
> >> 
> >>> converted all generic backend options into a dp_option array
> >>> instead?
> >> 
> >> This should be done separately. I filed a ticket for it: 
> >> https://fedorahosted.org/sssd/ticket/2141
> >> 
> >> Michal
> > 
> > Stephen, do you agree that converting the generic options into
> > dp_option array would be better than putting a single timeout into
> > be_ctx ?
> > 
> > If so, I would accept Michal's patch for now and convert the
> > options in a later release.
> 
> Yes, I think that's probably a good move. Ack to this patch for now.

Pushed to master.



More information about the sssd-devel mailing list