efried | edmondsw: You should have a look at https://review.openstack.org/#/c/614643/ at some point. It's merging already, but for your awareness. We also need to port it OOT, which I guess I can do rn. | 14:21 |
---|---|---|
edmondsw | efried looking | 14:22 |
edmondsw | how did both [1] and [2] merge? Shouldn't the second of those have failed to pass gate checks? | 14:25 |
efried | why? | 14:25 |
efried | oh, because of the new param | 14:26 |
efried | no, because that kwarg is only added if upt raises ReshapeNeeded | 14:26 |
efried | which there was no test path for with the powervm driver. | 14:26 |
efried | but this patch adds one | 14:27 |
edmondsw | I thought it would fail because the method signatures don't match | 14:28 |
edmondsw | regardless of whether anything actually tried to pass allocations to the method | 14:28 |
edmondsw | i.e., a subclass can't override a method in the parent without giving the same args, can it? | 14:28 |
efried | you mean from a rules-of-python perspective? Sure it can. Duck typing, and all that. | 14:30 |
openstackgerrit | Eric Fried proposed openstack/nova-powervm master: PowerVM upt parity for reshaper, DISK_GB reserved https://review.openstack.org/614781 | 14:30 |
efried | Here's the OOT patch ^ | 14:30 |
efried | Did the same change-id thing. | 14:30 |
efried | ...which is why you always add kwargs at the end when you change a method signature :) | 14:32 |
efried | *should* | 14:32 |
edmondsw | nah, the reason you do that is to not break callers who didn't use kwarg form | 14:34 |
edmondsw | (which they always should) | 14:34 |
edmondsw | at least that's the only reason I've ever heard | 14:34 |
edmondsw | I don't really see what duck typing has to do with it... but ok | 14:34 |
edmondsw | mdrabe see the new commit proposal when you get a chance ^ | 14:35 |
mdrabe | ack | 14:36 |
efried | edmondsw: Because the method isn't resolved until runtime. And there's absolutely nothing that cross-checks signatures. When the method runs, the call is against whatever version of the method was resolved. And as long as the form of the call gels with the form of the resolved method (in this case, the allocations kwarg omitted) it works just fine. | 14:56 |
edmondsw | I'm just surprised that there is nothing cross-checking signatures. I could have sworn there was | 15:00 |
*** chhagarw has joined #openstack-powervm | 16:12 | |
efried | edmondsw, mdrabe: One of y'all want to +W https://review.openstack.org/#/c/614781/ ? CI just passed. | 16:17 |
mdrabe | efried: Done | 16:19 |
efried | mdrabe: thanks | 16:23 |
openstackgerrit | Merged openstack/nova-powervm master: PowerVM upt parity for reshaper, DISK_GB reserved https://review.openstack.org/614781 | 16:35 |
openstackgerrit | Merged openstack/nova-powervm master: Use tempfile for powervm config drive https://review.openstack.org/613342 | 17:06 |
*** chhagarw has quit IRC | 18:51 |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!