Earlier  
Posted Nick Remark
#openstack-nova - 2017-06-27
14:49:12 dansmith how does claiming in a filter make that better?
14:49:23 edleafe dansmith: not "in a filter".
14:49:28 jaypipes edleafe: there definitely *will* be races for the same host. And the manager will be the thing responsible for retrying those claim attempts on the same or an alternate matching host.
14:49:37 dansmith jaypipes: right
14:49:39 edleafe dansmith: it's the final step in the process
14:49:47 dansmith edleafe: in the manager.
14:50:00 edleafe ok, fine, fine
14:50:26 edleafe it just looks to me like you're trading one entanglement for another..
14:50:32 edleafe But don't let that slow you down
14:50:44 jaypipes ..
14:50:51 dansmith I don't even know how to respond to that
14:50:58 jaypipes I'm not going to.
14:51:02 edleafe you don't have to
14:51:08 edleafe I've stated my concerns
14:51:20 jaypipes I'll go back to trying to clean up the proposed code and add commentary to it.
14:51:30 dansmith jaypipes: before you do, we need to have a serious talk
14:51:37 edleafe if you still want to go down that route, I'll accept that
14:51:53 jaypipes dansmith: yes?
14:52:09 dansmith jaypipes: a couple months ago you started doing this trailing paren-on-a-line-by-itself thing, which I argued about, but let you continue for some large function calls in tests
14:52:22 dansmith jaypipes: but this patch has them all over the place and for calls with only a few parameters that could be a couple lines
14:52:35 dansmith jaypipes: _none_ of our other code looks like that and it's an abrupt change recently
14:52:50 dansmith it looks very different to everything else and it really (really) bugs me
14:53:28 jaypipes dansmith: k, I'll restyle
14:53:43 dansmith jaypipes: I'd *really* appreciate it :)
14:54:58 jaypipes dansmith: just for the parens, right? not the dict notation?
14:55:21 dansmith jaypipes: the parens are what make my eyes bleed, but give me a dict notation example
14:55:45 dansmith if you mean just ending on a line with } that's fine
14:55:54 jaypipes yeah
14:56:10 jaypipes k, will change.
14:56:17 dansmith yeah, we do that everywhere and that's cool
14:56:39 jaypipes dansmith: no prob, will fix.
14:56:45 jaypipes dansmith: just for the record, I find this:
14:56:47 jaypipes resource_query = ",".join(
14:56:47 jaypipes sorted("%s:%s" % (rc, amount)
14:56:47 jaypipes for (rc, amount) in resources.items())
14:56:47 jaypipes )
14:56:55 dansmith le barf
14:56:59 jaypipes easier to read than this:
14:56:59 jaypipes resource_query = ",".join(sorted("%s:%s" % (rc, amount)
14:57:00 jaypipes for (rc, amount) in resources.items()))
14:57:10 dansmith you can make it three lines,
14:57:19 dansmith just don't put the trailing paren on a line by itself
14:57:30 jaypipes loud and clear sir.
14:57:34 dansmith but even that is slightly less smelly than:
14:57:37 dansmith myfunc(
14:57:40 dansmith foo, bar,
14:57:41 dansmith )
14:57:45 dansmith le double barf ^
14:57:49 jaypipes heh
15:03:56 openstackgerrit Andreas Scheuring proposed openstack/nova master: DO NOT MERGE! Dummy patch https://review.openstack.org/478186
15:08:31 efried dansmith +1, thanks for bringing a hammer to that party.
15:09:09 dansmith efried: interventions are sometimes necessary to correct misbehavior
15:09:48 dansmith I learned that from TLC
15:09:54 efried The more you know
15:10:15 dansmith the L used to stand for learning, but now it stands for something else like "look-at-the-freaks"
15:11:34 jaypipes dansmith: https://review.openstack.org/#/c/475448/13/nova/tests/functional/api/openstack/placement/fixtures.py
15:11:35 efried Never had the cable, so I'll have to take your word for it. (I have absolutely no trouble wasting enough time with Netflix and Amazon Prime.)
15:11:56 jaypipes dansmith: do you want me to de-parens those fixutres like on line 262?
15:13:09 dansmith jaypipes: yes, gross
15:13:30 jaypipes dansmith: ok.
15:13:55 jaypipes dansmith: so it's really just the lone paren on a line that you don't like, right?
15:14:25 dansmith jaypipes: and the trailing comma in a call that goes with it
15:14:37 efried I'll restate dislike for the trailing comma, which loses its benefit (two-line change becomes one-line change) if you pull the paren back.
15:14:40 efried yeah, what he said.
15:15:07 dansmith jaypipes: I don't really see the need to break every call into a multiline one-per-line verbose list either, but.. that's less smelly than the lone paren and trailing comma thing
15:15:14 dansmith for long ones, or ones with a very long parameter name, sure
15:15:30 jaypipes different style, is all.
15:15:37 jaypipes but I'll work with you.
15:15:49 dansmith jaypipes: I know, and we do some of that in other code.. we do not, however, do the trailing comma and lone paren thing :)
15:16:03 openstackgerrit Eric Fried proposed openstack/nova master: Add formatting to scheduling activity diagram https://review.openstack.org/476204
15:16:55 jaypipes dansmith, efried: you guys would hate Golang then :)
15:16:57 efried I personally dig the one-per-line thing for really long param lists. I'll take the vspace hit for readability there.
15:17:18 dansmith jaypipes: it's consistency with the rest of our code that I care about
15:17:24 jaypipes understood.
15:17:29 dansmith jaypipes: if all our code looked like that it would be fine (ugly, but fine)
15:17:50 dansmith efried: for long param lists, I do it myself sometimes, yeah
15:25:35 jaypipes dansmith: I'\
15:25:35 jaypipes m goin\
15:25:35 jaypipes g to start codi\
15:25:35 jaypipes ng like this just fo\
15:25:35 jaypipes r you.
15:25:53 dansmith jaypipes: I don't hate that, but it's not our style, so I'll still -1 it :)
15:25:58 jaypipes lol
15:26:09 dansmith er wait, you probably didn't get that, let me try again:
15:26:11 dansmith jaypipes: I don't hate that, but it's not our style, so I'll still -1 it :
15:26:12 dansmith )
15:26:13 dansmith there
15:26:21 dansmith FTFY
15:26:54 jaypipes rofl
15:27:39 jaypipes dansmith: you missed the comma at the end of the first line.
15:28:49 jaypipes dansmith: you're my little exorcist today.
15:29:15 dansmith jaypipes: dude, https://review.openstack.org/#/c/476630/6/nova/tests/unit/scheduler/weights/test_weights_affinity.py on line 50 and below
15:29:22 dansmith why do you hate parens all the sudden?
15:29:30 dansmith stop torturing parens, jaypipes
15:30:35 jaypipes dansmith: alignment is very important to me.
15:31:26 openstackgerrit Merged openstack/python-novaclient master: Updated from global requirements https://review.openstack.org/478083
15:31:31 dansmith jaypipes: you can align those without putting parens on their own lines
15:31:37 jaypipes dansmith: but I'm gonna rework that patch actually to reduce the amount of code change as much as possible, even if it means some hackery.
15:31:41 dansmith you've actually increased indent there for no reason :)
15:32:22 dansmith jaypipes: okay, it is a lot of change, but I'm not super opposed to the way it is, FWIW

Earlier   Later