| veebers | thomi: hey, you have a moment? | 02:42 |
|---|---|---|
| thomi | veebers: depends | 02:43 |
| thomi | what's up? | 02:43 |
| thomi | (btw, I've semi-officially started in CI now) | 02:43 |
| veebers | thomi: ah awesome, congrats :-) | 02:43 |
| veebers | 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:44 |
| veebers | I see in the gatekeeper testing a bunch of errors that seem to stem from CPOs that multi inherit. | 02:45 |
| veebers | 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:45 |
| veebers | thomi: so I was hoping for some advice on how to further debug this. | 02:46 |
| thomi | heh... multi-inheritance almost certainly won't work for CPOs | 02:46 |
| thomi | why do people feel the need to multi-inherit CPOs? | 02:47 |
| veebers | current culprit: class ContactListPage(_common.PageWithHeader, _common.PageWithBottomEdge): | 02:47 |
| thomi | yeah you can't do that | 02:47 |
| thomi | whoever did that needs a slapping :D | 02:47 |
| thomi | PageWith*** are both CPOs? | 02:47 |
| * veebers double checks | 02:48 | |
| thomi | if you want to do that, make those classes mixin classes | 02:48 |
| veebers | thomi: affirm | 02:48 |
| thomi | but you can only derive from your emulator_base exactly once in the class heirarchy | 02:48 |
| veebers | 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:49 |
| thomi | one sec | 02:50 |
| veebers | 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:50 |
| thomi | veebers: to answer your first question... yes, with a metaclass you can | 02:55 |
| thomi | you can walk up the inheritance tree and inspect everything there | 02:55 |
| thomi | WRT your second comment - it might have worked, FCVO "work" | 02:55 |
| thomi | I doubt, for example, you could have passed the child class as an object to select_* | 02:56 |
| veebers | 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 |
| thomi | something like that | 02:56 |
| veebers | ok, makes sense | 02:56 |
| veebers | 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 |
| thomi | remind me what our fix was, and what precipitated it? | 02:57 |
| veebers | 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:57 |
| veebers | thomi: this fix, where the extension classes weren't always being used in a class: | 02:58 |
| veebers | https://code.launchpad.net/~veebers/autopilot/fixing-extension-classes-and-test/+merge/246962 | 02:58 |
| veebers | The proposal I had today was pretty much: __bases__ = tuple(set(base).union(set(extensions))) | 02:58 |
| thomi | veebers: why doesn't that work? | 02:59 |
| veebers | 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) | 02:59 |
| thomi | what's the error message? | 03:00 |
| veebers | AttributeError: Class 'ContactListPage' has no attribute 'open_contact' | 03:00 |
| thomi | what's the difference in __bases__ with, and without your proposal? | 03:01 |
| veebers | 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:01 |
| veebers | I suspect something is happening earlier on that dirties something to make it misbehave | 03:02 |
| thomi | how do you mean 'changes'? a different VDO function is called? | 03:02 |
| veebers | thomi: yeah | 03:02 |
| thomi | what's the new function it's calling? | 03:02 |
| veebers | thomi: If I do: print(inspect.getsource(proxy_class_dict['ContactListPage'].validate_dbus_object)) | 03:02 |
| veebers | the first time around it's fine, but after that it's calling something like: return (name == b'webbrowser-app' and . . . | 03:03 |
| veebers | thomi: fyi: http://paste.ubuntu.com/10365925/ | 03:03 |
| veebers | that's 2 calls to _try_custom_proxy_classes (the ?? note the different calls) for the path "......PageStack/PageWrapper/ContactListPage" | 03:04 |
| thomi | this is *exactly* what I'd expect to happen when multiple inheritance is used | 03:04 |
| thomi | veebers: can you check the content of __bases__ in each case? | 03:05 |
| veebers | 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 |
| thomi | I bet __bases__ will reveal all :D | 03:05 |
| veebers | thomi: hmm, yeah sure. Give me a couple of minutes to hack that up and get details | 03:05 |
| veebers | thomi: also, if you're EOD we can pick this up tomorrow if you like | 03:06 |
| thomi | I'll be on for a bit longer | 03:06 |
| veebers | gah, now it's refusing to work at all :-| Sorry, just a couple more minutes :-) | 03:10 |
| veebers | thomi: looks like you were right :-) http://paste.ubuntu.com/10366010/ | 03:12 |
| veebers | s for source, b for bases, 3 calls there in the log | 03:12 |
| thomi | are you really surprised? :P | 03:12 |
| veebers | nope | 03:13 |
| thomi | so, someone's messing up __bases__ causing your MRO to go haywire | 03:13 |
| thomi | there's only like 2-3 places where that can happen | 03:13 |
| thomi | so I don't think it'll be hard to find | 03:13 |
| thomi | I suspect that new code that gets the bases is to blame | 03:13 |
| veebers | thomi: the new code being the stuff that I added today? | 03:14 |
| thomi | veebers: if not that, then in the same general vicinity, yeah | 03:14 |
| thomi | but I didn't mean to disparage your code :P | 03:14 |
| veebers | thomi: the order of __bases__ matters right, that's what the MRO is? | 03:14 |
| thomi | veebers: the order matters, yeah | 03:15 |
| thomi | the MRO is more than that though | 03:15 |
| thomi | the rules are somewhat non-trivial | 03:15 |
| veebers | thomi: heh, I didn't see any disparaging there | 03:15 |
| thomi | but for our purposes, the CPO-parent should be first | 03:15 |
| veebers | ok, so I need to do more than just set union the 2 bases ther | 03:15 |
| thomi | everything else (mixin classes) later | 03:15 |
| thomi | well... | 03:15 |
| veebers | yeah, that's what I had in mind. I'll hack around on that | 03:15 |
| thomi | you probably won't have any problems, but ideally, yeah | 03:16 |
| veebers | thomi: sweet, I'll work on that now. Thanks for the help :-) | 03:17 |
| thomi | no worries | 03:17 |
| thomi | I think tomorrow I'll withdraw myself from qa-related IRC channels BTW | 03:18 |
| veebers | thomi: ah ok, it won't stop me bothering you for help though ;-) | 03:18 |
| thomi | heh | 03:19 |
| === vrruiz_ is now known as rvr | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!