On 07/15/2013 04:41 PM, 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.
Hm, right, this should make the upgrade script a lot simpler - it would just remove all psearch or zone_refresh references. > > Another option is to release new bind-dyndb-ldap to Fedora 19 and change > default values to 'psearch yes' right now. Do you agree? Looks OK to me and would let us avoid doing any additional upgrade process for Fedora 20 - are you planning to do a Fedora 19 release any time soon? If yes, we can do the changes we are talking about in next 3.2.x release. Martin _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel