| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-06-27 | |||
| 11:26:43 | artom | The hyperv tests are... weird | |
| 11:36:23 | artom | As far as I can tell everything, including parameters passed to methods, and said methods return values, are mocked out | |
| 11:36:38 | artom | I'm not sure it's testing anything substantial | |
| 11:41:33 | openstackgerrit | Rodolfo Alonso Hernandez proposed openstack/os-vif master: Use ``assert_has_calls`` to check function calls https://review.openstack.org/477469 | |
| 11:49:21 | openstackgerrit | OpenStack Proposal Bot proposed openstack/nova master: Imported Translations from Zanata https://review.openstack.org/477091 | |
| 11:56:03 | openstackgerrit | Gao Yuan proposed openstack/nova master: Remove mox from nova.tests.unit.virt.test_block_device https://review.openstack.org/477904 | |
| 12:05:37 | bhagyashri_s | johnthetubaguy: Hi, Waiting for long time, Could you please review patch https://review.openstack.org/#/c/409644 ? address all review comment. Thank you. | |
| 12:05:59 | artom | alex_xu, hi, questions about your feedback in https://review.openstack.org/#/c/376021/71/nova/tests/unit/api/openstack/compute/test_attach_interfaces.py@251 | |
| 12:07:06 | artom | Having those tests in InterfaceAttachTestsV21 allowed me to use all the scaffolding that's already there. Should the new class inherit from InterfaceAttachTestsV21? That would replicate a lot of tests needlessly | |
| 12:08:21 | artom | If it doesn't inherit, we have to copy-paste said scaffolding | |
| 12:08:33 | artom | Well, maybe not the *entire* scaffolding | |
| 12:08:40 | artom | Let me see what I can get away with | |
| 12:08:48 | artom | Err, thanks for being my rubber duck :/ | |
| 12:09:18 | artom | And if I was talking to myself the whole time and no one's here, even better, it means I haven't wasted anyone's time :) | |
| 12:15:17 | alex_xu | artom: we can create a new class, and you needn't copy too much code, since your test is only about schema validation, so the api method will fail at very early stage, so you needn't mock or stub_out anything | |
| 12:16:33 | alex_xu | artom: the new class just inherit from test.NoDBTestCase | |
| 12:16:51 | openstackgerrit | OpenStack Proposal Bot proposed openstack/nova master: Updated from global requirements https://review.openstack.org/477982 | |
| 12:17:26 | artom | alex_xu, yep, that totally works, thanks :) | |
| 12:17:36 | alex_xu | artom: np :) | |
| 12:19:45 | openstackgerrit | OpenStack Proposal Bot proposed openstack/os-vif master: Updated from global requirements https://review.openstack.org/467173 | |
| 12:21:51 | jaypipes | ralonsoh: heya | |
| 12:22:10 | openstackgerrit | OpenStack Proposal Bot proposed openstack/python-novaclient master: Updated from global requirements https://review.openstack.org/478083 | |
| 12:24:39 | jaypipes | ralonsoh: did you need me for something? | |
| 12:27:09 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: Delete disk metadata when detaching volume https://review.openstack.org/435082 | |
| 12:27:10 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: API support for tagged device attachment https://review.openstack.org/376021 | |
| 12:34:42 | ralonsoh | jaypipes: hi, I left a comment in https://review.openstack.org/#/c/476612/ | |
| 12:34:54 | ralonsoh | jaypipes: about the abstraction of the ovsdb access | |
| 12:38:44 | jaypipes | ralonsoh: ah, gotcha | |
| 12:47:44 | sfinucan | sdague, alex_xu | |
| 12:47:49 | sfinucan | Can I kill this docs? https://docs.openstack.org/developer/nova/api_plugins.html | |
| 12:47:51 | sfinucan | *doc | |
| 12:48:45 | alex_xu | sfinucan: yea, but probably need new doc to teach people how to add new API to the nova | |
| 12:49:00 | sfinucan | Well, we don't do it as plugins anymore, right? | |
| 12:49:28 | sfinucan | alex_xu: Is this what you're looking for? https://docs.openstack.org/developer/nova/api_microversion_dev.html | |
| 12:55:19 | alex_xu | sfinucan: yea, although it is specific to the microversion | |
| 12:56:31 | sfinucan | alex_xu: Is there any other way to extend the API than with a microversion now? | |
| 12:56:41 | sfinucan | I thought we didn't let people extend the API anymore? | |
| 12:57:11 | sdague | sfinucan: we don't | |
| 12:57:21 | alex_xu | sfinucan: yes, that doc is for the nova developer add API to the upstream | |
| 12:57:32 | sdague | however the api_plugins page is the only one that talks about the how you add new bits of api code | |
| 12:57:38 | sfinucan | Ohhh, is it now | |
| 12:57:50 | sdague | the microversion page is just about changing responses in existing ones | |
| 12:57:56 | sfinucan | OK, I won't delete it then | |
| 12:58:05 | sdague | it definitely needs a rewrite | |
| 12:58:07 | sfinucan | But I think it will need to be reworked | |
| 12:58:08 | sfinucan | yup | |
| 12:58:11 | sfinucan | I'll include a TODO | |
| 12:58:25 | sdague | ++ | |
| 12:58:31 | alex_xu | yea, probably we need to mention this new route list https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/routes.py | |
| 12:58:48 | sfinucan | indeed | |
| 13:11:06 | jaypipes | bauzas, edleafe, Guest63234: will be doing my best to push revisions on the allocation requests series while on a 5-hour long sales call today. :( | |
| 13:12:42 | openstackgerrit | Alex Szarka proposed openstack/nova master: Refactor init_instance:resume_guests_state https://review.openstack.org/349582 | |
| 13:16:00 | edleafe | jaypipes: ugh | |
| 13:16:07 | jaypipes | edleafe: yeah. | |
| 13:16:13 | edleafe | jaypipes: will reply to your reply on the patch | |
| 13:16:17 | jaypipes | edleafe: regarding that... | |
| 13:16:48 | jaypipes | edleafe: so like I mentioned on my comment, I actually tried that approach first and ended up having to change just as many lines eventually in the unit tests. | |
| 13:17:12 | jaypipes | edleafe: It really comes down to whether we want the filter scheduler driver to be the thing that claims resources or not. | |
| 13:17:39 | jaypipes | edleafe: or the manager class. | |
| 13:18:16 | edleafe | jaypipes: can you explain why we can't have chance scheduler do claims? And I also thought that we weren't going to sidetrack this for chance scheduler | |
| 13:18:21 | jaypipes | edleafe: my objective was to make as few changes to the filter scheduler driver class as possible. | |
| 13:18:40 | edleafe | jaypipes: because if there is no claim made when a compute node gets a build request, it will claim | |
| 13:18:44 | jaypipes | edleafe: chance scheduler doesn't know uuids. | |
| 13:19:27 | jaypipes | edleafe: it's more than the chance scheduler. it's a separation of concerns thing. | |
| 13:19:47 | jaypipes | edleafe: I'm trying to leave the filter scheduler driver doing filtering and weighing. | |
| 13:19:57 | jaypipes | edleafe: and have something else (the manager) do the claiming. | |
| 13:20:22 | edleafe | jaypipes: but let's focus on the concept I pointed out: changing method sigs to pass compute uuids instead of just getting them when the host is updated from the compute | |
| 13:20:46 | edleafe | IOW, the value is already there | |
| 13:21:10 | jaypipes | edleafe: I was changing the method signature of the *returned* value so that the manager could do the claiming and not have to do everything in _schedule() | |
| 13:21:16 | mriedem | artom: was it intentional to not restrict block device tags to be unique for multiple bdms on the same instance? | |
| 13:21:21 | mriedem | we have no unique constraint for that | |
| 13:23:17 | artom | mriedem, yeah | |
| 13:23:40 | artom | mriedem, I don't see a use case for it either, but there might be a reason for it | |
| 13:23:59 | artom | For instance, two disks that have the same purpose get the same tag | |
| 13:24:03 | edleafe | jaypipes: I'll have to look more at that part. I was focusing on the changes to HostState | |
| 13:24:26 | edleafe | jaypipes: I am concerned how to select a host without first making a successful claim | |
| 13:24:38 | artom | mriedem, there was no real argument for or against, so by default it's allowed | |
| 13:24:50 | artom | Same for nics | |
| 13:24:55 | artom | And there it makes even less sense | |
| 13:25:07 | artom | Unless you're going to bond two nics and want them tagged the same? I dunno | |
| 13:25:12 | jaypipes | edleafe: we only make the claim after the scheduler filters and weighers have all run against the host states | |
| 13:26:06 | mriedem | artom: pl | |
| 13:26:08 | mriedem | *ok | |
| 13:26:19 | bhagyashri_s | johnthetubaguy: Hi, waiting for some inputs. | |
| 13:26:54 | artom | mriedem, plus I think semantically most people wouldn't expect a tag to be unique | |
| 13:27:02 | artom | When you think of tag clouds and such | |
| 13:27:16 | artom | You're not surprised when many elements share one or more tags | |
| 13:33:53 | alex_xu | artom: I guess you forget to set microversion on req obj https://review.openstack.org/#/c/376021/72/nova/tests/unit/api/openstack/compute/test_attach_interfaces.py@480 | |
| 13:34:46 | mriedem | Vek: do we expect DeprecatedAction to always issue a warning even if you don't specify the arg? https://review.openstack.org/#/c/477212/ | |
| 13:35:38 | artom | alex_xu, did I? | |
| 13:36:01 | alex_xu | artom: yea, those test should against on the 2.49 | |
| 13:36:02 | artom | alex_xu, I ran those tests locally and they all passed... because 'tag' isn't a valid parameter in anything < 2.49 | |
| 13:36:06 | artom | Doh | |
| 13:36:26 | alex_xu | artom: https://review.openstack.org/#/c/376021/72/nova/tests/unit/api/openstack/compute/test_attach_interfaces.py@516 this should use self.req | |
| 13:36:30 | artom | alex_xu, can you leave a comment and I'll fix | |
| 13:36:37 | alex_xu | artom: ok | |
| 13:37:20 | artom | alex_xu, thanks :) | |
| 13:37:47 | edleafe | jaypipes: I read ahead to the other PSs in the series to understand. It appears that select_destinations will return num_instances hosts to the manager - without claiming first. Right? | |
| 13:37:54 | edleafe | jaypipes: That means that there aren't any backup choices to select. The big gain we were hoping to get is when there is a race and the claim fails, the scheduler can simply select the next host in the weighed list. Having the manager make the claim means we lose that. | |
| 13:38:10 | openstackgerrit | Rodolfo Alonso Hernandez proposed openstack/os-vif master: Use ``assert_has_calls`` to check function calls https://review.openstack.org/477469 | |