| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-04-12 | |||
| 14:55:23 | fried_rice | 1055 of them are W504 | |
| 14:55:37 | tssurya | dansmith: yeah you have a valid point, its just that the behaviour of the command seemed weird when its all about discovering hosts | |
| 14:59:31 | dansmith | tbh, I think what melwitt has is probably what I would want.. we retain the existing behavior of returning nonzero for the duplicate case, we just get a nice message about why | |
| 14:59:39 | dansmith | no behavioral change at all, just easier for the humans | |
| 14:59:57 | dansmith | we can change the behavior too, but that means deployment people likely have to change their stuff | |
| 15:01:00 | tssurya | yea return code wise no behaviour change, just that now the command stops because it found an already discovered host :D | |
| 15:01:01 | melwitt | yeah, that's what I was thinking with return 1, was today it traces and returns nonzero, so keep the behavior, just warn about it instead of tracing | |
| 15:01:14 | dansmith | tssurya: it stopped before | |
| 15:01:14 | tssurya | but I guess I am okay with it considering we run it as a periodic task | |
| 15:01:25 | melwitt | I hadn't been thinking about the backwards (from the way my brain works) way that it currently returns values | |
| 15:01:28 | dansmith | tssurya: cern runs it as a periodic? | |
| 15:01:43 | tssurya | dansmith: no | |
| 15:01:47 | dansmith | tssurya: okay good :D | |
| 15:01:51 | tssurya | I meant in general :) | |
| 15:01:55 | dansmith | gotcha | |
| 15:02:15 | dansmith | melwitt: yeah, I think the return codes are a bit wonky, but what you have is an improvement for humans | |
| 15:02:30 | melwitt | tssurya: yeah, today the command stops because it raises an exception all the way up | |
| 15:03:37 | dansmith | melwitt: does this also continue to raise an exception for the periodic? | |
| 15:03:54 | dansmith | melwitt: because I thought we were going to catch and log a warning for the periodic case to avoid the trace in the logs | |
| 15:04:36 | melwitt | it would raise HostMappingExists yeah. I missed the point about catching and logging for periodic only | |
| 15:04:38 | dansmith | or did you want to do that in a separate patch | |
| 15:04:39 | dansmith | ? | |
| 15:05:03 | dansmith | don't you think that's better than a trace? LOG.warning('This should only be enabled when ...') | |
| 15:05:09 | tssurya | melwitt, dansmith: yeah ok thanks I am aboard the plan then | |
| 15:06:17 | melwitt | dansmith: yeah I think that's better, I just missed it and need to add it. what do you mean about "this should only be enabled when" though? | |
| 15:06:26 | tssurya | melwitt: let's also add something in the nova-manage docs like a fat note | |
| 15:06:43 | dansmith | like "this should only be enabled on a single scheduler" or something | |
| 15:06:57 | melwitt | dansmith: I'm concerned about mschuppert's comment here though https://bugzilla.redhat.com/show_bug.cgi?id=1698630#c7 does this mean he expects it to return zero in the dupe case? | |
| 15:06:58 | openstack | bugzilla.redhat.com bug 1698630 in openstack-tripleo-heat-templates "deployment fails running nova_cellv2_discover_hosts process with a duplicate key" [High,New] - Assigned to owalsh | |
| 15:07:10 | melwitt | dansmith: ack got it | |
| 15:07:42 | melwitt | tssurya: I think that docstring gets put in the docs right? let me check actually | |
| 15:08:07 | dansmith | melwitt: I dunno what of that you think is related to his expectation of the return value | |
| 15:08:29 | dansmith | melwitt: but if he's expecting something that isn't in line with the current behavior, he's probably expecting the wrong thing :) | |
| 15:08:34 | melwitt | "With the proposed change to nova to just report a warning we'll not fail in that case." | |
| 15:08:46 | dansmith | *my* concern about that comment is that he's talking about running this *on* the computes | |
| 15:09:13 | owalsh | dansmith: on one compute | |
| 15:09:16 | dansmith | melwitt: oh is he talking about your proposed change/ | |
| 15:09:27 | dansmith | owalsh: doesn't matter.. any compute :) | |
| 15:09:29 | owalsh | melwitt: he's assuming rc==0, yes | |
| 15:09:33 | melwitt | I assumed so. I had linked it on the BZ | |
| 15:09:42 | owalsh | dansmith: yea... not how I did it initially but reasons... | |
| 15:10:23 | dansmith | owalsh: that's unfortunate.. giving computes credentials to the api DB just to run this thing the controllers should do.. do those creds persist on the compute? | |
| 15:10:47 | dansmith | melwitt: okay I hadn't read it that way.. so he's just misunderstanding what your patch is doing you mean'? | |
| 15:11:09 | melwitt | I'm concerned that he is, yeah | |
| 15:11:43 | melwitt | tssurya: you're right, I need to add to the docs separately, the docstrings don't go in there automatically http://logs.openstack.org/47/651947/2/check/openstack-tox-docs/d6d7085/html/cli/nova-manage.html | |
| 15:11:48 | dansmith | melwitt: but he's quoting from owalsh who says they'll use a retry loop, which is fine yeah? | |
| 15:12:49 | melwitt | yeah, I guess so. I was hyperfocusing on the reply and not the quote | |
| 15:13:03 | owalsh | dansmith: not sure where the creds are coming from now actually... previously when I moved this to the computes I embedded them in the script and unlink the script as soon as it was run to avoid leaving creds around | |
| 15:13:16 | dansmith | owalsh: okay | |
| 15:13:31 | owalsh | dansmith: but it's been rewritten since, I'll take a look | |
| 15:15:50 | dansmith | owalsh: "talking to the api database" is something we generally expect a compute not to ever do.. I guess in this case you're using it as the admin node since you're just taking action on that one, | |
| 15:16:06 | dansmith | but in ansible lingo, I would "delegate" the discovery task to one of the controller nodes | |
| 15:16:47 | owalsh | dansmith: yea, the reasons for this were that the controller nodes were being excluded from tripleo scale outs to speed things up, so it had to be on a compute... there isn't anything else | |
| 15:17:49 | dansmith | right, because tripleo/heat reasons yeah? If it was pure ansible delegating a single admin command to one controller node after you're done would be easy, but I know that heat probably makes that hard | |
| 15:18:25 | owalsh | might be possible now that tripleo == heat generating ansible playbook, but not sure if it would backport | |
| 15:27:28 | fried_rice | finucannot: a little sphinx help? | |
| 15:27:35 | finucannot | shoot | |
| 15:27:55 | fried_rice | finucannot: I'm in a spec | |
| 15:28:03 | fried_rice | I want to make a link that gets referenced from a couple of places | |
| 15:28:18 | fried_rice | .. `some text`: https://... | |
| 15:28:18 | fried_rice | so it's like | |
| 15:28:28 | fried_rice | but in another place I want to use different text | |
| 15:28:41 | finucannot | :ref:`the text you want <the-anchor-name>` | |
| 15:28:42 | fried_rice | I tried `different text <some text>`_ | |
| 15:28:53 | finucannot | .. _the-anchor-name: https://... | |
| 15:28:57 | finucannot | Oh/ | |
| 15:28:58 | fried_rice | yeah, I tried :ref:`different text <some text>` | |
| 15:29:00 | finucannot | *? | |
| 15:29:01 | fried_rice | it didn't work | |
| 15:29:08 | finucannot | Hmm | |
| 15:29:22 | fried_rice | .. _`some text`: https:// | |
| 15:29:22 | fried_rice | sorry, yes | |
| 15:29:26 | fried_rice | forgot the _ originally | |
| 15:29:33 | fried_rice | The `some text`_ link works fine | |
| 15:29:39 | fried_rice | it's the "different text" I can't suss. | |
| 15:35:03 | finucannot | fried_rice: Ah, yeah, ref is for internal cross-referencing. My bad. | |
| 15:35:23 | finucannot | fried_rice: It's ugly, but I think the only way is to do something like this | |
| 15:35:43 | finucannot | `some text`_ and `some other text`_ | |
| 15:35:55 | finucannot | .. _some text: https://... | |
| 15:36:20 | finucannot | .. _`some other text`: `some text`_ | |
| 15:36:25 | finucannot | which doesn't really help :/ | |
| 15:37:47 | fried_rice | finucannot: It helps a little I guess. Point being to only specify the https://... in one place. | |
| 15:37:57 | finucannot | fried_rice: Wait, no, got it. `some other text <some text_>`_ | |
| 15:38:14 | finucannot | You need the first underscore to covert that to reference, otherwise it's taken literally | |
| 15:38:30 | finucannot | i.e. you'll end up at '/some text' | |
| 15:38:50 | fried_rice | yeah, that was the result I was getting. Trying.... | |
| 15:39:02 | finucannot | Or from earlier, `the text you want <the-anchor-name_>`_ | |
| 15:39:47 | fried_rice | finucannot: \o/ works! finuCAN! | |
| 15:40:06 | finucannot | Side note: 'sphinx-quickstart' in 2.0 has much less options and is now generating non-crufty conf.py files. Hurrah! | |
| 15:40:23 | fried_rice | what's that? | |
| 15:40:46 | finucannot | It's a tool Sphinx provides to bootstrap a new documentation project | |
| 15:41:05 | finucannot | tox has a similar one (tox-quickstart) inspired by Sphinx | |
| 15:41:18 | fried_rice | ah, cool | |
| 15:50:50 | zigo | mriedem: I proposed the same kind of change on python-novaclient too, but it looks like nobody was paying much attention to Python 3 at the time, and my patch got rejected there, when it was accepted in osc. I use exclusively osc anyway, so I didn't insist much ! :) | |
| 15:51:12 | zigo | Probably more out of lazyness, sorry for that. | |
| 15:52:11 | openstackgerrit | Theodoros Tsioutsias proposed openstack/nova master: Add instance hard delete https://review.openstack.org/570202 | |
| 15:52:12 | openstackgerrit | Theodoros Tsioutsias proposed openstack/nova master: Enable rebuild for instances in cell0 https://review.openstack.org/570203 | |
| 15:52:12 | openstackgerrit | Theodoros Tsioutsias proposed openstack/nova master: Add requested_networks to RequestSpec https://review.openstack.org/570201 | |
| 15:53:56 | openstackgerrit | Eric Fried proposed openstack/nova-specs master: Updates to image-metadata-prefiltering spec https://review.openstack.org/652101 | |
| 15:54:48 | fried_rice | sean-k-mooney, dansmith, jaypipes, alex_xu, finucannot: ^ | |