| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-01-04 | |||
| 16:43:07 | mriedem | dansmith: there will be a thrilling cells v2 meeting in 17 minutes yes? | |
| 16:43:34 | mriedem | dare i say titilating | |
| 16:44:09 | edleafe | cdent: 0, False, empty string/dict/list/tuple/set/etc are all boolean False, so you don't test for == 0, or == {}, or the like | |
| 16:44:43 | cdent | edleafe: I understand that, and that's my point: you _are_ testing for 0 here. Not False or empty | |
| 16:45:03 | openstackgerrit | Sylvain Bauza proposed openstack/nova: Expose a REST API for a specific list of RPs https://review.openstack.org/392569 | |
| 16:45:06 | cdent | because the fields definition has already established that used must be an int | |
| 16:45:10 | dansmith | mriedem: figured yeah | |
| 16:45:17 | edleafe | cdent: if you are getting unexpected parameter types here, you have much bigger problems | |
| 16:45:46 | bauzas | jaypipes: mriedem: dansmith: others, wanna bite https://review.openstack.org/392569 ? | |
| 16:46:07 | cdent | edleafe: hmm, I think we are miscommunicating a bit. At what stage of the process are ovo field types validated? | |
| 16:46:56 | mriedem | bauzas: it's in a tab | |
| 16:47:05 | cdent | if it is _before_ create is called, then self.used is always an int, and therefore the most limited and explicit (and thus readable) conditional is self.used == 0 | |
| 16:47:17 | bauzas | mriedem: just added a new test, so Ctrl-R is needed :) | |
| 16:47:21 | cdent | edleafe: that's all I'm saying | |
| 16:47:45 | mriedem | tdurakov: pkoniszewski: it would be awesome if we had a diagram of the call stack during live migration so i didn't have to relearn it everytime i'm reviewing something like this https://review.openstack.org/#/c/140045/51/nova/virt/hyperv/livemigrationops.py | |
| 16:47:53 | edleafe | cdent: if it could be an unexpected type, then you should do the type checking sepately | |
| 16:48:11 | cdent | yes, we agree on that | |
| 16:48:38 | edleafe | cdent: but if it's the correct type, then == 0 is ugly | |
| 16:48:45 | cdent | no, it's explicit | |
| 16:49:13 | sdague_ | mriedem: +A | |
| 16:49:45 | cdent | edleafe: but since that's aesthetics, I guess we should really just try to answer if we need to worry about negative numbers. I can't remember the answer to that. | |
| 16:49:56 | bauzas | cdent: edleafe: HTH https://github.com/openstack/oslo.versionedobjects/blob/master/oslo_versionedobjects/base.py#L70-L72 | |
| 16:50:52 | edleafe | bauzas: so it's enforced when the value is set, not when it's saved | |
| 16:51:03 | bauzas | indeed | |
| 16:51:09 | bauzas | easy to test | |
| 16:51:26 | openstackgerrit | Ed Leafe proposed openstack/nova: Do not post allocations that are zero https://review.openstack.org/407180 | |
| 16:51:36 | edleafe | cdent: ^^ try again :) | |
| 16:52:00 | cdent | now reason isn't perfect :) | |
| 16:53:35 | bauzas | cdent: edleafe: FWIW http://paste.openstack.org/show/593884/ | |
| 16:54:12 | edleafe | bauzas: thx | |
| 16:54:21 | edleafe | cdent: what's wrong with the reason?? | |
| 16:54:34 | cdent | "allocations of 0 resources used not allowed'" | |
| 16:54:46 | cdent | doesn't account for the negative handling you've just added | |
| 16:54:55 | cdent | but the :) was there for a reason | |
| 16:55:11 | cdent | I don't really care, but if we're striving for perfection... | |
| 16:55:21 | edleafe | cdent: ah, you meant in the error messag | |
| 16:55:30 | edleafe | I thought you meant the comment that I changed | |
| 16:57:04 | openstackgerrit | Ed Leafe proposed openstack/nova: Do not post allocations that are zero https://review.openstack.org/407180 | |
| 16:57:13 | edleafe | cdent: and you know I can't leave something like that there. ^^ | |
| 16:57:27 | cdent | I have become your enabler | |
| 17:00:37 | mdbooth | Could we please bang out these final 3 py3 fixes for libvirt: https://review.openstack.org/#/c/403423/ https://review.openstack.org/#/c/403419/ https://review.openstack.org/#/c/403431/ | |
| 17:00:46 | mdbooth | They've all had multiple rounds of review and rebases | |
| 17:01:09 | raj_singh | sfinucan: https://review.openstack.org/#/c/397774/ looks good to me, but because I fixed bunch of stuff in this I want johnthetubaguy to review it. | |
| 17:01:16 | mdbooth | It would be nice to get these done and forget about them | |
| 17:02:09 | sfinucan | mdbooth: Oops, forgot about those. I'll take a look shortly | |
| 17:02:32 | mdbooth | sfinucan: Thanks. Looks like you commented on the first on Monday? | |
| 17:02:41 | sfinucan | raj_singh: Aye, sounds fair. You know more about that than me though, so a +1 wouldn't go astray | |
| 17:03:00 | sfinucan | mdbooth: I did, but I'd lost the context. Not going to hold it up for my one comment | |
| 17:03:22 | mdbooth | Good, because I don't know the answer :) | |
| 17:06:50 | mdbooth | sfinucan: They mutually conflict (in tests-py3.txt), btw, so at least one of them will have to be remunged whatever. | |
| 17:06:59 | edleafe | cdent: did you mean 'persevering' or 'preseverating'? :) | |
| 17:07:18 | mdbooth | preseverating sounds wonderful | |
| 17:07:31 | cdent | heh, well played | |
| 17:07:32 | mdbooth | and potentially dangerous | |
| 17:07:57 | cdent | how about both? | |
| 17:14:54 | openstackgerrit | Rafael Folco proposed openstack/nova: DELETE all inventory for a resource provider https://review.openstack.org/416669 | |
| 17:16:54 | openstackgerrit | Dan Peschman proposed openstack/nova: No API cell up-call to delete consoleauth tokens https://review.openstack.org/415922 | |
| 17:22:43 | mriedem | woot https://blueprints.launchpad.net/nova/+spec/consistent-query-parameters-validation is complete | |
| 17:22:50 | mriedem | alex_xu: nice work | |
| 17:25:58 | openstackgerrit | Chris Dent proposed openstack/nova: [placement] Add a bit about extraction plans to placement_dev https://review.openstack.org/416676 | |
| 17:27:44 | openstackgerrit | Rafael Folco proposed openstack/nova: DELETE all inventory for a resource provider https://review.openstack.org/416669 | |
| 17:28:50 | cdent | mriedem: is this still on your radar? https://review.openstack.org/#/c/414230/ (more logging on rp update failures) | |
| 17:29:09 | cdent | If not, I can probably pick it up | |
| 17:29:26 | mriedem | cdent: i'm not actively on it right now so feel free | |
| 17:29:38 | openstackgerrit | Gábor Antal proposed openstack/nova: Transform instance.volume_attach notification https://review.openstack.org/401992 | |
| 17:29:42 | cdent | ✔ | |
| 17:30:57 | openstackgerrit | Rafael Folco proposed openstack/nova: DELETE all inventory for a resource provider https://review.openstack.org/416669 | |
| 17:33:14 | openstackgerrit | Rafael Folco proposed openstack/nova: DELETE all inventory for a resource provider https://review.openstack.org/416669 | |
| 17:35:47 | openstackgerrit | Chris Dent proposed openstack/nova: WIP: Add more debug logging on RP inventory update failures https://review.openstack.org/414230 | |
| 17:36:15 | openstackgerrit | Gábor Antal proposed openstack/nova: Transform instance.volume_detach notification https://review.openstack.org/408676 | |
| 17:37:01 | openstackgerrit | Gleb Stepanov proposed openstack/python-novaclient: Refactor novaclient code https://review.openstack.org/416682 | |
| 17:39:17 | sfinucan | mdbooth: Done (all trivial reapprovals bar the first). You just need to rope someone like japyipes in now :) | |
| 17:44:32 | openstackgerrit | Balazs Gibizer proposed openstack/nova: Transform aggregate.create notification https://review.openstack.org/394511 | |
| 17:45:13 | openstackgerrit | Balazs Gibizer proposed openstack/nova: Transform aggregate.delete notification https://review.openstack.org/394512 | |
| 17:45:40 | openstackgerrit | Balazs Gibizer proposed openstack/nova: Add aggregate notification related enum values https://review.openstack.org/396209 | |
| 17:45:53 | openstackgerrit | Balazs Gibizer proposed openstack/nova: Transform aggregate.add_host notification https://review.openstack.org/396210 | |
| 17:46:04 | openstackgerrit | Balazs Gibizer proposed openstack/nova: Transform aggregate.remove_host notification https://review.openstack.org/396211 | |
| 17:53:05 | johnthetubaguy | gibi: I wasn't sure about this one, I am curious about your thoughts around that: https://review.openstack.org/#/c/408689/ | |
| 18:09:46 | sean-k-mooney | mriedem: QQ when https://review.openstack.org/#/c/411988/ is merged to relese the new version of os-vif will it automatically update the requirements repo or should i submit a patch to the requiremets repo to do that? | |
| 18:10:48 | openstackgerrit | Dan Smith proposed openstack/nova: Fix non-parameterized service id in hypervisors sample tests https://review.openstack.org/410471 | |
| 18:10:48 | openstackgerrit | Dan Smith proposed openstack/nova: Multicell support for instance listing https://review.openstack.org/396775 | |
| 18:10:49 | openstackgerrit | Dan Smith proposed openstack/nova: WIP: Make api_samples tests use simple cell environment https://review.openstack.org/410473 | |
| 18:10:49 | openstackgerrit | Dan Smith proposed openstack/nova: Make the SingleCellSimple fixture a little more comprehensive https://review.openstack.org/410472 | |
| 18:10:53 | openstackgerrit | Dan Smith proposed openstack/nova: WIP Move instance creation to conductor https://review.openstack.org/319379 | |
| 18:11:40 | mriedem | sean-k-mooney: you have to submit a patch to raise the minimum required version of os-vif in global-requirements yourself | |
| 18:12:02 | mriedem | the upper-constraints update happens automatically, but if you're going to bump g-r you might as well bump u-c in the same patch - i think you'd have to anyway for the tests to pass | |
| 18:13:37 | sean-k-mooney | mriedem: thanks. im going to follow up with sfinucan again tomorow and see if we can get his outstanding patch merged so that the release can go ahead. | |
| 18:14:06 | sean-k-mooney | ill update the patche to the release repo and to the requirements repo once that is done | |
| 18:14:14 | sean-k-mooney | sorry for the delay | |
| 18:14:22 | mriedem | sean-k-mooney: cool i was going to ask you if his change at the bottom of the series was really required or not | |
| 18:15:06 | sean-k-mooney | we could release without it but he is fixing an issue on an inplace upgrade where the mtu chages. | |
| 18:15:23 | sean-k-mooney | it would be nice to have but not strictly required. | |
| 18:15:54 | mriedem | ok we still have some time | |
| 18:17:58 | sean-k-mooney | i assume the os-vif versions are fix on stable branch correct. e.g. they cannot be raised after the sable/branch is made ? | |
| 18:18:49 | mriedem | sean-k-mooney: correct | |
| 18:19:00 | mriedem | well the minimum can't | |
| 18:19:58 | sean-k-mooney | and uppper contratiants ? i assume they are also fixed or can the be extended if the new version is backwards compatible? | |
| 18:21:51 | sean-k-mooney | we dont need to extend them currently i just wanted to know if we had a reason too could we in the future. | |
| 18:23:05 | openstackgerrit | Sujitha proposed openstack/nova: Document testing process for zero downtime upgrade https://review.openstack.org/395269 | |
| 18:23:59 | mriedem | sean-k-mooney: i don't think upper-constraints is auto-updated on stable branches | |