On Tue, 2012-04-10 at 11:03 -0400, John Dennis wrote: > On 04/06/2012 10:11 AM, John Dennis wrote: > > On 04/06/2012 04:40 AM, Martin Kosek wrote: > >> 1) We still crash when the parameter is empty. We may want to make it > >> required (the same fix Rob did for cert rejection reason): > >> > >> # echo "secret123" | ipa migrate-ds ldap://vm-054.idm.lab.bos.redhat.com > >> --with-compat --base-dn="dc=greyoak,dc=com" --user-container= > >> ipa: ERROR: cannot connect to > >> u'http://vm-022.idm.lab.bos.redhat.com/ipa/xml': Internal Server Error > > > > Good point, will fix. > > > >> 2) Do you think it would make sense to create a special Param for DN? > >> Its quite general type and I bet there are other Params that could use > >> DN instead of Str. It could look like that: > >> DN('usercontainer?', > >> rdn=True,<<<< can be RDN, not DN > > > > Yes, I considered introducing a new DN parameter type as well. I think > > this is a good approach and will have payoff down the road. I will make > > that change. However I'm inclined to introduce both a DN parameter and a > > RDN parameter, they really are different entities, if you use a flag to > > indicate you require a RDN then you lose the type of the parameter, it > > could be either, and it really should be one or the other and knowing > > which it is has value. > > > >> 3) We should not restrict users from passing a user/group container with > >> more than one RDN: > > > > Yeah, I wasn't too sure about that. The parameter distinctly called for > > an RDN, but it seemed to me it should support any container below the > > base, which would make it a DN not a RDN. > > > > FWIW, a DN is an ordered sequence of 1 or more RDN's. DN's do not have > > to be "absolute", they can be any subset of a "path sequence". A DN > > with exactly one RDN is equivalent to an RDN, a special case). > > > > I will change the parameter to be a DN since that is what was the > > original intent. > > > > All good suggestions, a revised patch will follow shortly. > > Hmm... I'm rethinking this. The suggestion of adding a new DN parameter > is the right thing to do and I tried to implement it, but as I was > working I realized I needed to touch a fair amount of code to support > the new parameter type and to modify multiple places in the migration > code to work with the new type. That's more code changes at the very > last minute for this release than I'm comfortable with, we're too close > the freeze date for invasive modifications.
Ok, I agree with this approach. Lets fix just points 1) and 3) in my intial review. Martin _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel