| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-06-02 | |||
| 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 | |
| 15:19:41 | cdent | yes | |
| 15:19:49 | sfinucan | *too, even | |
| 15:21:06 | sfinucan | But the Python 2 variant is better than nothing, for some of the more complex stuff (like nearly everything in virt/libvirt) | |
| 15:25:42 | mriedem | cfriesen_: pep8 on your chnage | |
| 15:25:43 | mriedem | *change | |
| 15:33:57 | openstackgerrit | Jay Pipes proposed openstack/nova master: Sync os-traits to Traits database https://review.openstack.org/469578 | |
| 15:38:54 | mriedem | dansmith: heh i guess this is a duplicate https://review.openstack.org/#/c/460377/ of https://review.openstack.org/#/c/431722/ | |
| 15:38:56 | mriedem | drats | |
| 15:39:22 | dansmith | mriedem: it's all yours :) | |
| 15:40:42 | mriedem | looks like the only difference is you dropped the online data migrations, and in mine i think i was asking myself about that | |
| 15:40:46 | mriedem | b/c of skip level upgrading people | |
| 15:41:09 | mriedem | https://review.openstack.org/#/c/460377/6/nova/objects/flavor.py@648 | |
| 15:41:34 | dansmith | mriedem: as I've said before, I don't think leaving migrations in the tree for that reason makes sense | |
| 15:41:46 | mriedem | i tend to agree | |
| 15:44:21 | mriedem | well i reckon i better sort out if there are any other differences here and drop mine - although i am of course partial to my commit message | |
| 15:48:34 | dansmith | mriedem: I just abandoned mine so you can't do that :P | |
| 15:49:23 | mriedem | hells bells | |
| 15:49:32 | mriedem | i figured i'd just rebase yours | |
| 15:55:55 | openstackgerrit | Merged openstack/nova master: Connecting Nova to DRBD storage nodes directly https://review.openstack.org/256292 | |
| 16:00:27 | leakypipes | mriedem, melwitt, dansmith, bauzas: easy wins (placement-api-ref) starting here: https://review.openstack.org/#/c/452707/ | |
| 16:00:43 | dansmith | dibs | |
| 16:01:10 | mriedem | i'm gonna clean up cfriesen_'s change and +2 before it lunch | |
| 16:01:13 | mriedem | so maybe post-lunch | |
| 16:03:27 | dansmith | leakypipes: are those not rendered in the docs job anywhere? | |
| 16:04:16 | cdent | dansmith: different doc job, let me find link | |
| 16:04:25 | leakypipes | cdent: thx | |
| 16:04:36 | dansmith | found it | |
| 16:04:40 | cdent | gate-placement-api-ref-nv | |
| 16:06:31 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: Convert instance to primitive for metadata cache https://review.openstack.org/469978 | |
| 16:07:46 | dansmith | we're really spoiling our users with this api_ref nonsense | |
| 16:09:43 | leakypipes | dansmith: lol | |
| 16:09:50 | mriedem | dansmith: don't worry, we still have plenty of config options that change the behavior of the API in ways not communicated, | |
| 16:09:58 | mriedem | so we can still screw them | |
| 16:10:06 | openstackgerrit | Matt Riedemann proposed openstack/nova master: show flavor info in server details https://review.openstack.org/434514 | |
| 16:10:50 | mriedem | ^ and below are +2'ed | |