[02:42] thomi: hey, you have a moment? [02:43] veebers: depends [02:43] what's up? [02:43] (btw, I've semi-officially started in CI now) [02:43] thomi: ah awesome, congrats :-) [02:44] thomi: I would like to bounce something off you. In autopilot, the change that we made for the extension classes in the proxy object creation (def _try_custom_proxy_classes) wasn't perfect [02:45] I see in the gatekeeper testing a bunch of errors that seem to stem from CPOs that multi inherit. [02:45] I tried what I thought would be a fix, but I'm getting really odd issues, for instance, the validate_dbus_object method gets swapped out at some stage to something completely different [02:46] thomi: so I was hoping for some advice on how to further debug this. [02:46] heh... multi-inheritance almost certainly won't work for CPOs [02:47] why do people feel the need to multi-inherit CPOs? [02:47] current culprit: class ContactListPage(_common.PageWithHeader, _common.PageWithBottomEdge): [02:47] yeah you can't do that [02:47] whoever did that needs a slapping :D [02:47] PageWith*** are both CPOs? [02:48] * veebers double checks [02:48] if you want to do that, make those classes mixin classes [02:48] thomi: affirm [02:48] but you can only derive from your emulator_base exactly once in the class heirarchy [02:49] thomi: hmm, ok. So now I have a couple more queries. Is there a way that we can determine that a cpo is attempting to use multiple inheritance? (so we can provide a sane warning) [02:50] one sec [02:50] secondly, this is now a larger issue as it means that application test code needs to change (because up until now this seems to have worked). [02:55] veebers: to answer your first question... yes, with a metaclass you can [02:55] you can walk up the inheritance tree and inspect everything there [02:55] WRT your second comment - it might have worked, FCVO "work" [02:56] I doubt, for example, you could have passed the child class as an object to select_* [02:56] ah right, walk the tree set a flag when finding something inherited from the CustomProxyBase, error if we attempt to set again or something [02:56] something like that [02:56] ok, makes sense [02:57] thomi: ok, so my current plan is to back out the fix that we did re: the extension classes, get the doco stuff and a couple of other fixes in [02:57] remind me what our fix was, and what precipitated it? [02:57] thomi: Then I'll get a proper fix for the base class stuff and send out a communication to the teams to make sure that the code is correct and ready [02:58] thomi: this fix, where the extension classes weren't always being used in a class: [02:58] https://code.launchpad.net/~veebers/autopilot/fixing-extension-classes-and-test/+merge/246962 [02:58] The proposal I had today was pretty much: __bases__ = tuple(set(base).union(set(extensions))) [02:59] veebers: why doesn't that work? [02:59] thomi: my proposal? I'm not entirely certain, but it's causing issues with the address book test I'm using to confirm (my failing test passes with it too) [03:00] what's the error message? [03:00] AttributeError: Class 'ContactListPage' has no attribute 'open_contact' [03:01] what's the difference in __bases__ with, and without your proposal? [03:01] thomi: the odd thing I'm seeing is that the validate_dbus_object method code changes so that possible_classes is always an empty list so never attempt to use the CPO version [03:02] I suspect something is happening earlier on that dirties something to make it misbehave [03:02] how do you mean 'changes'? a different VDO function is called? [03:02] thomi: yeah [03:02] what's the new function it's calling? [03:02] thomi: If I do: print(inspect.getsource(proxy_class_dict['ContactListPage'].validate_dbus_object)) [03:03] the first time around it's fine, but after that it's calling something like: return (name == b'webbrowser-app' and . . . [03:03] thomi: fyi: http://paste.ubuntu.com/10365925/ [03:04] that's 2 calls to _try_custom_proxy_classes (the ?? note the different calls) for the path "......PageStack/PageWrapper/ContactListPage" [03:04] this is *exactly* what I'd expect to happen when multiple inheritance is used [03:05] veebers: can you check the content of __bases__ in each case? [03:05] thomi: in a good or bad way? I'm not sure where the webapp stuff is coming from as this is the addressbook app tests and CPOs [03:05] I bet __bases__ will reveal all :D [03:05] thomi: hmm, yeah sure. Give me a couple of minutes to hack that up and get details [03:06] thomi: also, if you're EOD we can pick this up tomorrow if you like [03:06] I'll be on for a bit longer [03:10] gah, now it's refusing to work at all :-| Sorry, just a couple more minutes :-) [03:12] thomi: looks like you were right :-) http://paste.ubuntu.com/10366010/ [03:12] s for source, b for bases, 3 calls there in the log [03:12] are you really surprised? :P [03:13] nope [03:13] so, someone's messing up __bases__ causing your MRO to go haywire [03:13] there's only like 2-3 places where that can happen [03:13] so I don't think it'll be hard to find [03:13] I suspect that new code that gets the bases is to blame [03:14] thomi: the new code being the stuff that I added today? [03:14] veebers: if not that, then in the same general vicinity, yeah [03:14] but I didn't mean to disparage your code :P [03:14] thomi: the order of __bases__ matters right, that's what the MRO is? [03:15] veebers: the order matters, yeah [03:15] the MRO is more than that though [03:15] the rules are somewhat non-trivial [03:15] thomi: heh, I didn't see any disparaging there [03:15] but for our purposes, the CPO-parent should be first [03:15] ok, so I need to do more than just set union the 2 bases ther [03:15] everything else (mixin classes) later [03:15] well... [03:15] yeah, that's what I had in mind. I'll hack around on that [03:16] you probably won't have any problems, but ideally, yeah [03:17] thomi: sweet, I'll work on that now. Thanks for the help :-) [03:17] no worries [03:18] I think tomorrow I'll withdraw myself from qa-related IRC channels BTW [03:18] thomi: ah ok, it won't stop me bothering you for help though ;-) [03:19] heh === vrruiz_ is now known as rvr