| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-07-26 | |||
| 08:06:31 | kashyap | lyarwood: Sorry for baiting you in the morning. If you're still making coffee or something, please go on. This can wait. | |
| 08:06:34 | lyarwood | ^ search for noqa | |
| 08:07:14 | kashyap | lyarwood: Most excellent, thanks for the (non-null) pointer! | |
| 08:08:01 | lyarwood | kashyap: so unicode isn't a keyword in py3 but is in py2 | |
| 08:08:20 | kashyap | lyarwood: Ha! So we indeed need to retain that "# noqa" | |
| 08:08:31 | lyarwood | yup | |
| 08:09:05 | kashyap | lyarwood: Thanks, I was going a bit mental right in the morning | |
| 08:09:47 | kashyap | mdbooth: When you are about, per above discussion with lyarwood, we can't "undent" the "# noqa" here: https://github.com/openstack/nova/blob/master/nova/virt/libvirt/guest.py#L668 | |
| 08:11:38 | lyarwood | kashyap: well you can, the #noqa is just to catch the unicode issue | |
| 08:12:21 | lyarwood | anyway /me goes back to downstream paper work | |
| 08:12:27 | kashyap | lyarwood: Alright, I'll look into it. | |
| 08:12:36 | kashyap | mdbooth: Disregard the above for now; more to come soon in the review | |
| 08:29:30 | bauzas | re: https://github.com/openstack/nova/blob/master/nova/virt/libvirt/guest.py#L668 | |
| 08:29:37 | bauzas | kashyap: lyarwood: ^ | |
| 08:30:22 | bauzas | kashyap: lyarwood: there is a six method for it | |
| 08:30:34 | kashyap | bauzas: I see | |
| 08:30:39 | kashyap | bauzas: Got a link? | |
| 08:31:08 | bauzas | kashyap: https://pythonhosted.org/six/#six.text_type | |
| 08:35:43 | bauzas | kashyap: http://paste.openstack.org/show/726668/ | |
| 08:36:24 | bauzas | a 'kéké' in French is a 'lad' | |
| 08:38:00 | bauzas | kashyap: of course a bytestring is not unicode | |
| 08:38:13 | kashyap | Hehe | |
| 08:40:44 | kashyap | bauzas: Thanks! | |
| 08:43:05 | bauzas | kashyap: final explanation between bytestrings and unicode strings : http://paste.openstack.org/show/726669/ | |
| 08:43:38 | bauzas | using .decode() will translate it into unicode with the encoding you want | |
| 08:56:58 | kashyap | Nod | |
| 09:02:25 | mdbooth | kashyap: That's barely worthy of discussion :) Did I call it a nit? If not, I should have done. | |
| 09:06:39 | kashyap | mdbooth: Hehe, I just make sure every point a reviewer raises is addressed, and not coldly ignore :P | |
| 09:07:00 | kashyap | Address, if it makes sense, i.e. | |
| 09:07:24 | mdbooth | kashyap: Yeah. I couldn't even tell, by eyeball, if the line was short enough. | |
| 09:07:30 | mdbooth | But I thought it probably was. | |
| 09:08:42 | kashyap | mdbooth: Ah, by "undent", I actually thought you wanted me to use a different way than the "# noqa" special comment | |
| 09:09:05 | kashyap | Look for "# noqa" there: http://pep8.readthedocs.io/en/release-1.7.x/intro.html#error-codes | |
| 09:09:27 | mdbooth | I assumed it was there for line length? | |
| 09:09:32 | mdbooth | That's how it's normally used. | |
| 09:10:01 | kashyap | The line was 74 lines. | |
| 09:10:07 | kashyap | Err | |
| 09:10:09 | kashyap | 74 chars | |
| 09:10:18 | kashyap | So, it's within the range, though. | |
| 09:10:20 | mdbooth | Weird. Wonder what the pep8 issue was. | |
| 09:12:54 | kashyap | mdbooth: PEP8 issue was due me not knowing about the special comment "# noqa" (& accidentally removing it) | |
| 09:15:41 | lyarwood | mdbooth: unicode is only used in py2, pep8 now runs under py3 by default and without #noqa that fails, bauzas pointed to a six method to handle this better above. | |
| 09:16:58 | bauzas | lyarwood: mdbooth: tbh, we don't need to check the python version | |
| 09:17:22 | lyarwood | yup, just use six | |
| 09:18:32 | mdbooth | bauzas: We do need the python version check there, I think. | |
| 09:18:59 | bauzas | mdbooth: the problem is that libvirtd is only py2 right? | |
| 09:19:19 | mdbooth | bauzas: No, it's both. The problem is that it handles strings differently in py2 and py3. | |
| 09:19:20 | bauzas | mdbooth: so it's waiting for a byte string | |
| 09:19:39 | mdbooth | in py2 it *must* be byte string. In py3 it *must* be unicode. | |
| 09:19:40 | bauzas | mdbooth: so that's a libvirt problem | |
| 09:19:42 | mdbooth | Yeah. | |
| 09:19:54 | bauzas | grr | |
| 09:20:17 | bauzas | so, yeah we need to check the python version | |
| 09:20:32 | bauzas | I thought it was only need a byte string | |
| 09:20:53 | bauzas | that's a terrible interface | |
| 09:21:07 | mdbooth | In fairness it's a bug. | |
| 09:21:11 | bauzas | yeah | |
| 09:21:16 | bauzas | a libvirt bug | |
| 09:21:20 | mdbooth | But one which can't be fixed now. | |
| 09:21:30 | mdbooth | Indeed, a libvirt bug. | |
| 09:21:56 | bauzas | oh and FWIW, params.items() only works because that's checked for py2 | |
| 09:22:38 | mdbooth | IIRC params.items() is the recommended way to migration py2 -> py3 unless you really can't handle a list or a generator | |
| 09:24:01 | mdbooth | Yeah, params.items() would have the same behaviour there in py2 and py3. | |
| 09:25:25 | mdbooth | lyarwood: Chance of thunderstorms this afternoon, apparently \o/ | |
| 09:25:28 | mdbooth | But not for you | |
| 09:26:01 | bauzas | mdbooth: while py2 returns a dict, py3 returns a generator | |
| 09:26:19 | mdbooth | bauzas: Right, and they'll both do the same thing there. | |
| 09:32:42 | lyarwood | mdbooth: getting three days of rain from tomorrow so tis all good :) | |
| 10:11:43 | openstackgerrit | Merged openstack/nova master: ironic: Report resources as reserved when needed https://review.openstack.org/517921 | |
| 10:17:18 | kashyap | I thought you might be interested in getting banned! | |
| 10:18:45 | openstackgerrit | Takashi NATSUME proposed openstack/nova master: Adds view builders for keypairs controller https://review.openstack.org/347289 | |
| 10:20:50 | kashyap | mdbooth: I will leave the uncode / byte string change as-is. Not going to muck with it in this change. | |
| 10:21:43 | mdbooth | kashyap: Right. I was only ever suggesting a formatting change following a change of indentation :) | |
| 10:22:09 | mdbooth | kashyap: Changing the unicode thing in that change would be inappropriate. | |
| 10:22:19 | kashyap | mdbooth: I would have parsed the word "unindent" correctly. But you said: "undent", my brain went: "Can we remove this wrinkle / dent" :P | |
| 10:22:24 | kashyap | mdbooth: Absolutely | |
| 10:23:01 | kashyap | Okido, mooooooooooving on. | |
| 10:42:56 | openstackgerrit | Kashyap Chamarthy proposed openstack/nova master: libvirt: Remove usage of migrateToURI{2} APIs https://review.openstack.org/567258 | |
| 11:05:50 | openstackgerrit | Chris Dent proposed openstack/nova master: [placement] Retry allocation writes server side https://review.openstack.org/586048 | |
| 11:07:40 | openstackgerrit | Takashi NATSUME proposed openstack/nova master: [placement] api-ref: add traits parameter https://review.openstack.org/578048 | |
| 11:27:05 | openstackgerrit | Andrey Volkov proposed openstack/osc-placement master: Get resource provider by uuid or name https://review.openstack.org/527791 | |
| 12:00:19 | openstackgerrit | Andrey Volkov proposed openstack/osc-placement master: Provide a useful message in the case of 500-error https://review.openstack.org/586056 | |
| 12:01:23 | openstackgerrit | Andrey Volkov proposed openstack/osc-placement master: Provide a useful message in the case of 500-error https://review.openstack.org/586056 | |
| 12:23:00 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Use placement 1.28 in scheduler report client https://review.openstack.org/583667 | |
| 12:35:46 | openstackgerrit | OpenStack Release Bot proposed openstack/os-traits stable/rocky: Update .gitreview for stable/rocky https://review.openstack.org/586101 | |
| 12:35:48 | openstackgerrit | OpenStack Release Bot proposed openstack/os-traits stable/rocky: Update UPPER_CONSTRAINTS_FILE for stable/rocky https://review.openstack.org/586102 | |
| 12:35:50 | openstackgerrit | OpenStack Release Bot proposed openstack/os-traits master: Update reno for stable/rocky https://review.openstack.org/586103 | |
| 12:36:00 | openstackgerrit | OpenStack Release Bot proposed openstack/os-vif stable/rocky: Update .gitreview for stable/rocky https://review.openstack.org/586104 | |
| 12:36:02 | openstackgerrit | OpenStack Release Bot proposed openstack/os-vif stable/rocky: Update UPPER_CONSTRAINTS_FILE for stable/rocky https://review.openstack.org/586105 | |
| 12:36:05 | openstackgerrit | OpenStack Release Bot proposed openstack/os-vif master: Update reno for stable/rocky https://review.openstack.org/586106 | |
| 12:36:42 | openstackgerrit | OpenStack Release Bot proposed openstack/osc-placement stable/rocky: Update .gitreview for stable/rocky https://review.openstack.org/586113 | |
| 12:36:45 | openstackgerrit | OpenStack Release Bot proposed openstack/osc-placement stable/rocky: Update UPPER_CONSTRAINTS_FILE for stable/rocky https://review.openstack.org/586114 | |
| 12:39:08 | mriedem | gmann: i'm going to close out https://blueprints.launchpad.net/nova/+spec/api-extensions-merge-rocky for rocky which is a nice cut off point since what remains is the view builder stuff | |
| 12:39:12 | mriedem | i'll open a bp for stein | |
| 12:39:54 | gmann | mriedem: sure. Thanks | |
| 12:44:58 | mriedem | i need some core love on the last 2 changes of the port binding live migration series https://review.openstack.org/#/c/434870/ | |
| 12:45:10 | mriedem | the 2nd change has a +2 already | |
| 12:45:30 | mriedem | gibi: bauzas: dansmith: ^ | |
| 12:45:48 | gibi | mriedem: I'm in the process of reading it right now | |
| 12:46:03 | mriedem | cool, thanks | |