| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-06-02 | |||
| 14:00:54 | bauzas | mriedem: had a comment on https://review.openstack.org/#/c/256292/6 | |
| 14:02:49 | bauzas | mriedem: out of curiosity, do you know if Tooz driver is still a thing for our SG API or whether we should consider etcd ? | |
| 14:04:22 | openstackgerrit | Claudiu Belu proposed openstack/nova master: POC: tests: autospecs all the mock.patch usages https://review.openstack.org/447505 | |
| 14:07:29 | mlavalle | mriedem: hey. I am looking at https://bugs.launchpad.net/nova/+bug/1694417 | |
| 14:07:30 | openstack | Launchpad bug 1694417 in OpenStack Compute (nova) "dhcp_domain used when use_neutron is not set" [Undecided,Incomplete] | |
| 14:07:34 | claudiub | mriedem: hi. i've found a nice pretty little bug. :) https://review.openstack.org/#/c/447505/13/nova/virt/libvirt/driver.py | |
| 14:08:17 | mlavalle | mriedem: seems that for the time being, we should wait for reply from the submitter | |
| 14:09:02 | sfinucan | mriedem, melwitt: All ze comments in https://review.openstack.org/#/c/379524/ _should_ be addressed now, if you fancy taking a gander today | |
| 14:09:40 | sfinucan | bauzas: RE: comments in https://review.openstack.org/#/c/256292/6/nova/virt/libvirt/volume/drbd.py | |
| 14:09:50 | sfinucan | instance is automatically exposed by oslo.log | |
| 14:10:04 | sfinucan | by way of the default formatter | |
| 14:10:10 | bauzas | mmmm okay | |
| 14:10:29 | melwitt | yeah, you can do that on any log message | |
| 14:10:31 | sfinucan | at least, iirc. mriedem has suggested I do something similar a few times previously | |
| 14:10:37 | sfinucan | *prior? | |
| 14:13:10 | claudiub | mriedem: also, one question: the cinder 3rd party ci requirements state that "CI's do not need to kick off tests until Jenkin's has given a +1", is there a similar policy for nova as well? ( https://wiki.openstack.org/wiki/Cinder/tested-3rdParty-drivers#Third_Party_CI_Requirements ) | |
| 14:20:50 | efried | claudiub I have that same thought every couple months ;-) Problem is that it would make overall completion take longer. Which is the lesser evil? | |
| 14:23:27 | claudiub | efried: i guess it depends on the size of the ci. at some point, all the ci resources could be allocated / in use. anyways, even though a ci can be configured to wait for jenkins to vote +1, it can still be triggered manually via "check some-ci" if needed. | |
| 14:23:37 | mriedem | bauzas: we should probably deprecate/remove tooz service group api driver | |
| 14:23:40 | mriedem | bauzas: it's not tested, | |
| 14:23:45 | mriedem | and i don't know if anyone outside of yahoo was using it | |
| 14:23:51 | mriedem | might be worth asking harlowja | |
| 14:24:07 | mriedem | bauzas: etcd is probably a better idea, and is something we can test in the gate | |
| 14:24:19 | mriedem | like i the multinode job or nova-next job | |
| 14:24:21 | mriedem | *in | |
| 14:24:30 | efried | claudiub Looks like mriedem missed that one in https://review.openstack.org/#/c/452958/ -- you going to propose the fix? | |
| 14:25:10 | mriedem | bauzas: instance=instance is common in all of our logging :) | |
| 14:25:15 | claudiub | ah, sure. will do. | |
| 14:25:37 | mriedem | efried: claudiub: missed what? | |
| 14:25:56 | mriedem | claudiub: i already have a patch for that | |
| 14:26:05 | claudiub | ah, ok. | |
| 14:26:15 | mriedem | claudiub: efried: https://review.openstack.org/#/c/469958/ | |
| 14:26:21 | mriedem | bauzas: sfinucan: melwitt: dansmith: jaypipes: ^ | |
| 14:26:31 | mriedem | and https://review.openstack.org/#/c/469740/ | |
| 14:26:36 | mriedem | live migration rollback is borked w/o those two | |
| 14:26:52 | bauzas | mriedem: about instance=instance, sorry for my ignorance | |
| 14:27:16 | sfinucan | mriedem: In a meeting, but I'll look at them soon as I'm done | |
| 14:27:39 | efried | claudiub Perhaps you can suggest how to use autospec for https://review.openstack.org/#/c/469958/1/nova/tests/unit/virt/libvirt/test_driver.py ? | |
| 14:30:33 | efried | claudiub And https://review.openstack.org/#/c/469740/2/nova/tests/unit/compute/test_compute_mgr.py | |
| 14:32:19 | claudiub | efried: well, mock.patch does accept an "autospec" argument, but it doesn't work properly under the current release of mock. I have sent a bug report and bug fix for it: https://github.com/testing-cabal/mock/issues/396 | |
| 14:32:44 | claudiub | that should be the cleanest way; but it didn't get much attention unfortunately. | |
| 14:32:45 | efried | claudiub Riiight, I've been (sorta) following that. | |
| 14:35:09 | claudiub | actually, it seems the mock library hasn't been updated in 6 months. | |
| 14:56:04 | mriedem | rfolco: questions/issues in https://review.openstack.org/#/c/460147/ | |
| 14:56:06 | mriedem | cdent: ^ | |
| 14:56:42 | rfolco | mriedem, will address asap | |
| 14:57:01 | mriedem | i could be wrong | |
| 14:58:53 | cdent | mriedem: on which particular part? | |
| 14:59:59 | mriedem | any? | |
| 15:00:20 | mriedem | we used to check for 200 in the response from the 1.0 call | |
| 15:00:27 | mriedem | now we're checking a range, which is different and scary | |
| 15:00:32 | mriedem | and we're not handling 404 in case the RP is already gone | |
| 15:00:54 | cdent | the range thing is the generic version of "success" that is a mode of being a tolerant client. trendy | |
| 15:01:16 | cdent | the missing 404 is a good point, not sure if that's caught somewhere else or not, but very good of you to point it out | |
| 15:01:25 | cdent | probably means a missing gabbi test | |
| 15:01:45 | rfolco | I thought that was covered, would confirm | |
| 15:01:45 | mriedem | i expect the fallback code to be the same as before, and the range thing is different, which is why i flagged it | |
| 15:01:46 | cdent | s/gabbi/functional resource tracker/ | |
| 15:02:06 | mriedem | hard to review stuff like this if refactored parts are changed | |
| 15:02:19 | mriedem | if you want the range, do it in a preceeding change | |
| 15:02:22 | cdent | mriedem: i think an earlier review caused that change, opportunistic | |
| 15:02:31 | cdent | s/caused/ask for/ | |
| 15:02:32 | mriedem | *preceding | |
| 15:02:45 | mriedem | let's separate it | |
| 15:02:49 | cdent | sure | |
| 15:03:01 | cdent | rfolco: you on it? | |
| 15:03:24 | cdent | anybody have anything I don't already know about (he said tautologically) | |
| 15:07:11 | rfolco | mmedvede, cdent: thanks for reviewing. | |
| 15:07:32 | rfolco | oops mriedem | |
| 15:07:47 | rfolco | sorry mmedvede, used to call you on irc :) | |
| 15:08:44 | mmedvede | :) | |
| 15:09:32 | openstackgerrit | Merged openstack/nova master: Updated from global requirements https://review.openstack.org/470119 | |
| 15:09:38 | sfinucan | artom: Comments left on https://review.openstack.org/#/c/465205/. Happy to +2 if you can address em? | |
| 15:10:07 | artom | sfinucan, no, I shall sit here and pout until you +2 | |
| 15:10:19 | artom | sfinucan, in all serious, thanks for taking the time to review :) | |
| 15:10:25 | artom | Let me respin | |
| 15:10:32 | sfinucan | damn, pouting always gets me | |
| 15:10:50 | artom | Um, I... | |
| 15:11:20 | artom | I think you need to tell your bf/gf/wife/SO/whoever? | |
| 15:11:26 | artom | (I'm not taking any chances) | |
| 15:13:48 | sfinucan | mriedem: Never show mdbooth those two patches - you'll surely set him off on one of his days long Python type system rants | |
| 15:13:53 | sfinucan | And, reviewed. All g with me | |
| 15:14:16 | mriedem | sfinucan: the live migration ones? he already saw one of them the other night | |
| 15:14:23 | mriedem | lee invoked his name | |
| 15:14:31 | sfinucan | dammit, lee | |
| 15:14:44 | mriedem | the inability to autospec those tests is frustrating, i could just be missing something trivial, | |
| 15:14:55 | mriedem | but i spent an hour one night trying to get it working on the driver_detach one and gave up | |
| 15:15:48 | efried | claudiub Doesn't mock.patch autospec work in the decorator form? | |
| 15:16:57 | sfinucan | I think pylint would catch those issues (too many args/invalid kwargs for function) | |
| 15:17:11 | sfinucan | but it'd introduce so many false positives as to make it not worth effort | |
| 15:17:16 | sfinucan | mriedem: Ever seen mypy? | |
| 15:17:55 | mriedem | no | |
| 15:18:23 | sfinucan | worth checking it out. I'm thinking of integrating it into some of the virt/libvirt code | |
| 15:18:26 | cdent | do you like it sfinucan ? | |
| 15:18:38 | sfinucan | The Sphinx source uses it pretty much everywhere | |
| 15:18:42 | cdent | it seems simultaneously really useful but also noisy | |
| 15:18:55 | sfinucan | cdent: I like it in it's Python 3 form | |
| 15:19:04 | sfinucan | with proper annotations rather than comments | |
| 15:19:08 | cdent | and lack of noise was one of the things that keeps me loyal to python | |
| 15:19:35 | sfinucan | the opt-in nature of the whole thing is pretty attractive to | |