[Freeipa-devel] [PATCH] 891 drop has_upg() check

[Martin Kosek]

On Thu, 2011-10-13 at 15:28 -0400, Rob Crittenden wrote:
> Martin Kosek wrote:
> > On Thu, 2011-10-13 at 15:09 -0400, Rob Crittenden wrote:
> >> Rob Crittenden wrote:
> >>> Martin Kosek wrote:
> >>>> On Thu, 2011-10-13 at 11:01 -0400, Rob Crittenden wrote:
> >>>>> Martin Kosek wrote:
> >>>>>> On Wed, 2011-10-12 at 23:54 -0400, Rob Crittenden wrote:
> >>>>>>> The has_upg() check was created during a transition period for 389-ds.
> >>>>>>> It is no longer needed and is actually breaking things. The
> >>>>>>> location of
> >>>>>>> UPG template moved so it thinks the feature is not available. This is
> >>>>>>> making the primary user's group ipausers instead of the UPG.
> >>>>>>>
> >>>>>>> rob
> >>>>>>
> >>>>>> Shouldn't we remove has_managed_entries() and its use too? After
> >>>>>> all, we
> >>>>>> claim that this patch fixes #1242 which asks for has_managed_entries()
> >>>>>> removal.
> >>>>>>
> >>>>>> Martin
> >>>>>>
> >>>>>
> >>>>> Updated patch attached. It removes has_managed_entries().
> >>>>>
> >>>>> rob
> >>>>
> >>>> Looks good - there is just some leftover in the bottom of commit
> >>>> message, probably from patch squashing.
> >>>>
> >>>> However, I was thinking about has_upg() removal. Shouldn't we check if
> >>>> the UPG plugin is enabled (the same way we do in ipa-managed-entries)?
> >>>> Otherwise if the plugin is disabled and we would run user-add command
> >>>> without --noprivate option, we would set nonexistent GID for the user as
> >>>> the UPG wouldn't be created.
> >>>>
> >>>> Martin
> >>>>
> >>>
> >>> Ok, good point.
> >>>
> >>> I decided to just fix has_upg() for now.
> >>>
> >>> I'm caching the value so we don't have to do an extra search every
> >>> single time we add a user. I don't think this is the kind of thing that
> >>> is going to be turned on/off a lot (e.g. you'll turn it off and be done
> >>> with it).
> >>>
> >>> rob
> >>
> >> Updated patch to remove caching. Since the config is now replicated if
> >> an admin disables it they would quickly have to restart all Apache
> >> servers on all replicas which is bad.
> >>
> >> rob
> >>
> >
> > ipaserver/plugins/ldap2.py:723: [E0001] invalid syntax
> >
> > return = False? Really? :-)
> >
> > Martin
> >
> 
> I'm feeling very philosophical right now. To return or not return...
> 
> rob

Return. Always.

ACK and pushed to master, ipa-2-1.

Martin