On Mon, 2013-07-15 at 16:41 +0200, Petr Spacek wrote:
> On 15.7.2013 16:15, Simo Sorce wrote:
> > On Mon, 2013-07-15 at 15:57 +0200, Martin Kosek wrote:
> >> On 07/15/2013 03:44 PM, Petr Spacek wrote:
> >>> On 15.7.2013 15:31, Martin Kosek wrote:
> >>>> On 07/11/2013 05:10 PM, Tomas Babej wrote:
> >>>>> On Thursday 11 of July 2013 16:10:33 Ana Krivokapic wrote:
> >>>>>
> >>>>>> On 07/11/2013 11:20 AM, Tomas Babej wrote:
> >>>>>
> >>>>>>> boolean_var = {}
> >>>>>
> >>>>>>> - for var in ('persistent_search', 'serial_autoincrement'):
> >>>>>
> >>>>>>> + for var in ('serial_autoincrement'):
> >>>>>
> >>>>>> This won't work - a one element tuple needs a comma at the end:
> >>>>>
> >>>>>> ('serial_autoincrement', )
> >>>>>
> >>>>>>> boolean_var[var] = "yes" if getattr(self, var, False) else "no"
> >>>>>
> >>>>>>>
> >>>>>
> >>>>>>> self.sub_dict = dict(FQDN=self.fqdn,
> >>>>>
> >>>>>>> @@ -607,9 +604,8 @@ class BindInstance(service.Service):
> >>>>>
> >>>>>>> SUFFIX=self.suffix,
> >>>>>
> >>>>>>> OPTIONAL_NTP=optional_ntp,
> >>>>>
> >>>>>>> ZONEMGR=self.zonemgr,
> >>>>>
> >>>>>>> - ZONE_REFRESH=self.zone_refresh,
> >>>>>
> >>>>>>> IPA_CA_RECORD=ipa_ca,
> >>>>>
> >>>>>>> - PERSISTENT_SEARCH=boolean_var['persistent_search'],
> >>>>>
> >>>>>>> + PERSISTENT_SEARCH="yes",
> >>>>>
> >>>>>>> SERIAL_AUTOINCREMENT=boolean_var['serial_autoincrement'],)
> >>>>>
> >>>>>>
> >>>>>
> >>>>>> But anyway, I think this piece of code is unnecessarily complicated, I
> >>>>>> don't see
> >>>>>
> >>>>>> a need for the 'boolean_var' dict here. I would suggest replacing it
> >>>>>> with
> >>>>>
> >>>>>> something like:
> >>>>>
> >>>>>>
> >>>>>
> >>>>>> serial_autoincrement = "yes" if self.serial_autoincrement else "no"
> >>>>>
> >>>>>>
> >>>>>
> >>>>>> and then pass serial_autoincrement to self.sub_dict = dict(...)
> >>>>>
> >>>>>>
> >>>>>
> >>>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>> Attached patch refactored the relevant part of the code.
> >>>>>
> >>>>>
> >>>>>
> >>>>> Tomas
> >>>>>
> >>>>
> >>>> Thanks for patches! I am just thinking, should we also hide the
> >>>> respective
> >>>> option from ipa global DNS configuration? That's idnszonerefresh
> >>>> attribute.
> >>>>
> >>>> We may want to mark the attribute as invisible in CLI + remove it from
> >>>> Web UI.
> >>>> Petr - what is your take on this? Do you plan to remove idnszonerefresh
> >>>> attribute support in the future (Fedora 20) as persistent search will be
> >>>> mandatory in that time?
> >>>
> >>> Yes, you are right. We completely forgot to web UI. And yes - please
> >>> remove the
> >>> option from web UI.
> >>
> >> Ok, Tomas please do the changes as proposed above.
> >>
> >>>
> >>> The latest development shows that persistent search will be replaced by
> >>> RFC
> >>> 4533 (known as 'syncrepl'), but from user's point of view it doesn't
> >>> matter.
> >>> All options related to persistent search and zone_refresh will simply
> >>> disappear. Syncrepl itself doesn't require explicit configuration.
> >>
> >> Ah, so this means that "psearch" option will be also removed from
> >> bind-dyndb-ldap? In Fedora 19 we just plan to hard-code it to "yes", will
> >> that
> >> cause issues with Fedora 20? Should we already avoid using the "psearch"
> >> option
> >> and assume that bind-dyndb-ldap in Fedora 19 is using persistent search by
> >> default?
> >
> > Won't the new bind-dyndb-ldap simply ignore the psearch option when it
> > moves to syncrepl ?
>
> I can do it, but I think that cleanest way is to remove the 'psearch' option
> in upgrade script.
Sure, but if the upgrade, for whatever reason, fails to remove it
bind-dyndb-ldap should just ignore.
> Another option is to release new bind-dyndb-ldap to Fedora 19 and change
> default values to 'psearch yes' right now. Do you agree?
Too much churn, I think it is ok to change it when we are done with
syncrepl and upgrade config, with the fallback failsafe that even if
upgrade doesn't remove the option bind-dyndb-ldap will simply ignore it.
This will be safer even for people using stuff like cfengine/puppet to
manage configurations and not realizing we changed the conf on upgrade,
their confsystems will push again a conf file with psearch yes but
bnid-dyndb-ldap won't break.
Simo.
--
Simo Sorce * Red Hat, Inc * New York
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
