Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-01-04
15:27:37 johnthetubaguy (I thought it was the later)
15:27:48 bauzas mriedem: the latter yes
15:27:51 mriedem ok
15:28:28 bauzas (sorry, I'm awfully busy today, hopefully being less impacted tomorrow :/ )
15:28:45 mriedem np. on that note, i'm running to a meeting.
15:35:18 sdague cdent: are you fine with this - https://review.openstack.org/#/c/407180/6 ?
15:38:33 cdent sdague: I think the code has some stylistic issues, but functionally it's okay. I can go ahead and leave those comments now, but I wouldn't want it to block.
15:39:16 openstackgerrit Christopher Martin proposed openstack/nova: Bugfix: Changes to Instance Soft Delete https://review.openstack.org/386756
15:39:35 openstackgerrit Wang Qing wu proposed openstack/nova: WIP(2): Introduce base PowerVM driver (disk drv) https://review.openstack.org/409401
15:43:48 cdent sdague, edleafe: I've left those comments
15:56:16 openstackgerrit Gábor Antal proposed openstack/nova: Added missing decorator for instance.create.error https://review.openstack.org/416597
16:02:05 jaypipes edleafe: hey! you know I love my C code :)
16:09:54 cfriesen any chance of getting some reviewers for https://review.openstack.org/#/c/260339/ ? (It's not my patch, but it's a useful fix.) There's a note about a possible tempest race in the bug, so would we need to fix the tempest test before fixing nova?
16:10:44 openstackgerrit Gábor Antal proposed openstack/nova: Removed unnecessary parantheses and fixed formation https://review.openstack.org/370549
16:19:33 openstackgerrit melanie witt proposed openstack/nova: WIP Count resources to check quota for cells https://review.openstack.org/416521
16:20:42 edleafe cdent: Not sure what I was going for there, but yeah, it looks wrong. Will fix.
16:21:21 cdent ✔
16:30:14 openstackgerrit Gleb Stepanov proposed openstack/python-novaclient: Add evacuate host method https://review.openstack.org/349179
16:30:52 sfinucan mriedem: Boom https://review.openstack.org/#/c/416083
16:32:09 openstackgerrit Gábor Antal proposed openstack/nova: Removed unnecessary parantheses and fixed formation https://review.openstack.org/370549
16:32:33 sfinucan :P
16:32:42 openstackgerrit Wang Qing wu proposed openstack/nova: WIP(3): Introduce base PowerVM driver (console) https://review.openstack.org/409402
16:36:29 openstackgerrit Ed Leafe proposed openstack/nova: Do not post allocations that are zero https://review.openstack.org/407180
16:36:43 edleafe cdent: should be better ^^
16:37:29 cdent edleafe: decided you didn't like my concern about self.used?
16:38:57 edleafe cdent: you mean protecting against negative used values?
16:39:24 cdent yes, and also being explicit about the conditional being numeric
16:39:48 edleafe cdent: not sure I follow
16:40:39 cdent I just think that self.used == 0 or self.used <= 0 is more explicit/readable than not self.used
16:41:02 mriedem sdague: mtreinish: easy one https://review.openstack.org/#/c/416401/
16:41:06 mriedem sfinucan: thanks
16:41:09 edleafe cdent: self.used == 0 is major ugly in Python
16:41:27 edleafe <= 0 is another thing
16:41:31 mtreinish mriedem: +2
16:41:45 cdent but it is _exactly_ what you mean to be testing for there, especially based on the message raised if it is true
16:42:00 edleafe cdent: if you think we need to guard against negative values, I'll add <= 0
16:42:40 cdent a) just out of curiosity why is self.used == 0 ugly? b) I'm not sure if we do or not, I'm asking, not telling
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

Earlier   Later