On 02/08/2013 05:30 PM, Alexander Bokovoy wrote: > On Fri, 01 Feb 2013, Martin Kosek wrote: >> On 02/01/2013 03:55 PM, Alexander Bokovoy wrote: >>> On Tue, 29 Jan 2013, Martin Kosek wrote: >>>> trust_output_params = ( >>>> @@ -482,3 +499,158 @@ api.register(trust_mod) >>>> api.register(trust_del) >>>> api.register(trust_find) >>>> api.register(trust_show) >>>> + >>>> + >>>> +_trust_type_option = ( >>>> + StrEnum('trust_type', >>>> + cli_name='type', >>>> + label=_('Trust type (ad for Active Directory, default)'), >>>> + values=(u'ad',), >>>> + default=u'ad', >>>> + autofill=True, >>>> + ), >>>> +) >>> We already have various trust type definitions in the same file. Maybe >>> it makes sense to unify those somehow? >> >> Right, I unified those 2 separate trust_type option definitions. >> >>> >>>> + def get_dn(self, *keys, **kwargs): >>>> + trust_type = kwargs.get('trust_type') >>>> + if trust_type is None: >>>> + raise errors.RequirementError(name='trust_type') >>>> + if kwargs['trust_type'] == u'ad': >>> Perhaps better to define constants for the trust type values... >> >> I changed it a bit and now it uses a dict instead. I think its now more >> general >> and extensible. >> >>> >>>> + except ValueError: >>>> + # The search is performed for groups with "posixgroup" >>>> objectclass >>>> + # and not "ipausergroup" so that it can also match groups like >>>> + # "Default SMG Group" which does not have this objectclass. >>> 'Default SM_B_ Group' >> >> Fixed. >> >>> >>> Thanks for the unit tests too! >>> >> >> You are welcome! I also generated API.txt which I forgot to do last time. >> Updated patch attached. > ACK for the code but please add more documentation (below). > > Works like sharm. I tried also changing default fallback group to > some IPA group, then back to Default SMB Group and it worked well. Also > specifying non-existing group was noted and rejected. > > Please make sure to mention in the design page magic value 'Default SMB > Group' and also that you can use any group with 'posixgroup' > objectclass, and that 'Default SMB Group' is not visible through normal > IPA tools. > > We need to write better documentation (online help) for trustconfig-mod. > Basically, right now it helps no one to understand what is supposed to > be done here. > > Once help is added, ACK.
Thanks for the review! RFE updated with information you mentioned. I also added more info to trust online help (which you verified off-list). Pushed to master, ipa-3-1. Martin _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel