Re: [Freeipa-devel] [PATCH] jderose 053 XML-RPC signature change
John Dennis wrote: On 03/26/2010 09:22 AM, John Dennis wrote: On 03/26/2010 07:24 AM, Jason Gerard DeRose wrote: This quick patch changes the XML-RPC signature to match the complementary change being made in certmonger. The signature is now: [args, options] This doesn't yet include the [args, options, extra] change... that is coming in my rpcserver patch once it's done. But this provides what needed for current IPA<=> certmonger compatibility. NAK Is there a reason for the type inconsistency? Why is it a list in one case and a tuple in the other? I realize they'll both operate the same way but the inconsistency is confusing especially if there is no reason to use different type objects (e.g. no need for a mutable sequence). Jason has promised to fix the consistency issue in a subsequent patch so I'll ACK this now so it can get in. Ok, pushed to master rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] jderose 053 XML-RPC signature change
On 03/26/2010 09:22 AM, John Dennis wrote: On 03/26/2010 07:24 AM, Jason Gerard DeRose wrote: This quick patch changes the XML-RPC signature to match the complementary change being made in certmonger. The signature is now: [args, options] This doesn't yet include the [args, options, extra] change... that is coming in my rpcserver patch once it's done. But this provides what needed for current IPA<=> certmonger compatibility. NAK Is there a reason for the type inconsistency? Why is it a list in one case and a tuple in the other? I realize they'll both operate the same way but the inconsistency is confusing especially if there is no reason to use different type objects (e.g. no need for a mutable sequence). Jason has promised to fix the consistency issue in a subsequent patch so I'll ACK this now so it can get in. -- John Dennis Looking to carve out IT costs? www.redhat.com/carveoutcosts/ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] jderose 053 XML-RPC signature change
On 03/26/2010 05:18 PM, Jason Gerard DeRose wrote: On Fri, 2010-03-26 at 09:22 -0400, John Dennis wrote: On 03/26/2010 07:24 AM, Jason Gerard DeRose wrote: This quick patch changes the XML-RPC signature to match the complementary change being made in certmonger. The signature is now: [args, options] This doesn't yet include the [args, options, extra] change... that is coming in my rpcserver patch once it's done. But this provides what needed for current IPA<=> certmonger compatibility. NAK Is there a reason for the type inconsistency? Why is it a list in one case and a tuple in the other? I realize they'll both operate the same way but the inconsistency is confusing especially if there is no reason to use different type objects (e.g. no need for a mutable sequence). We use lists and tuples interchangeability. Tuple are nice because they aren't mutable and are a bit more efficient in terms of memory use, but both json.loads() and xmlrpclib.loads() return lists. My general plan has been to move to using just lists. So if the plan is to converge on just using lists then what better time to start than now? json.dumps() and xmlrpclib.dumps() also treat tuples and lists the same... both are serialized to a list type. So there's no type change of any consequence in this patch. As I said, I know they are interchangeable in most cases. When I read code and see differing usage I presume there is a reason and I go looking for the reason so that I can follow the correct usage model. If there is no actual difference then I've wasted time trying to understand why things are different. Consistency has value especially in a huge body of code contributed to by many developers. I realize in this particular case I'm nit-picking and I fully understand the API's of xmlrpclib and json but I'm going to push for things like consistency, removal of dead code and no code duplication whenever I see it. All those things just make it harder to understand the code. It's part of the reason we do patch reviews along with finding logic errors. -- John Dennis Looking to carve out IT costs? www.redhat.com/carveoutcosts/ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] jderose 053 XML-RPC signature change
On Fri, 2010-03-26 at 09:22 -0400, John Dennis wrote: > On 03/26/2010 07:24 AM, Jason Gerard DeRose wrote: > > This quick patch changes the XML-RPC signature to match the > > complementary change being made in certmonger. > > > > The signature is now: > > > > [args, options] > > > > This doesn't yet include the [args, options, extra] change... that is > > coming in my rpcserver patch once it's done. But this provides what > > needed for current IPA<=> certmonger compatibility. > > NAK > > Is there a reason for the type inconsistency? Why is it a list in one > case and a tuple in the other? I realize they'll both operate the same > way but the inconsistency is confusing especially if there is no reason > to use different type objects (e.g. no need for a mutable sequence). We use lists and tuples interchangeability. Tuple are nice because they aren't mutable and are a bit more efficient in terms of memory use, but both json.loads() and xmlrpclib.loads() return lists. My general plan has been to move to using just lists. json.dumps() and xmlrpclib.dumps() also treat tuples and lists the same... both are serialized to a list type. So there's no type change of any consequence in this patch. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] jderose 053 XML-RPC signature change
On 03/26/2010 07:24 AM, Jason Gerard DeRose wrote: This quick patch changes the XML-RPC signature to match the complementary change being made in certmonger. The signature is now: [args, options] This doesn't yet include the [args, options, extra] change... that is coming in my rpcserver patch once it's done. But this provides what needed for current IPA<=> certmonger compatibility. NAK Is there a reason for the type inconsistency? Why is it a list in one case and a tuple in the other? I realize they'll both operate the same way but the inconsistency is confusing especially if there is no reason to use different type objects (e.g. no need for a mutable sequence). -- John Dennis Looking to carve out IT costs? www.redhat.com/carveoutcosts/ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
