| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-05-29 | |||
| 14:52:17 | mriedem | it is using the queens constraints, it's just not obvious at all from the logs | |
| 14:52:19 | sean-k-mooney | sphix was the package that was breaking the follow up package | |
| 14:53:23 | sean-k-mooney | it was previously uncpped v7 or this failed but v8 works after it was rebased on the constraitns patch https://review.opendev.org/#/c/655694/8 | |
| 14:54:17 | mriedem | yup i can see http://logs.openstack.org/94/655694/7/check/openstack-tox-py35/c0ad3dc/tox/py35-3.log | |
| 14:55:27 | mriedem | ok +W on the queens uc change | |
| 14:57:24 | sean-k-mooney | thanks when you have time it would be good to merge the bugfix backport too but as i said it still need to be backported on stien and rock + we wont be doing a release for a while so it not urgent | |
| 14:57:41 | mriedem | yeah i'm looking, creating the missing release series in lp for os-vif first | |
| 14:57:43 | mriedem | so i can target the bug | |
| 15:01:59 | sean-k-mooney | on a related note im planning to do an intermediate releaseof os-vif before m2 so that any nova features that need the new release can technicall merge before m2 assuming they are ready | |
| 15:03:11 | sean-k-mooney | but i dont think there is enough currently to justify a release so it will be at leat another couple of weeks. | |
| 15:03:56 | mriedem | sean-k-mooney: ralonsoh: i don't know if this came up in review on the change on master, but we should probably use mock_open here https://review.opendev.org/#/c/655678/3/vif_plug_ovs/tests/unit/test_linux_net.py@83 | |
| 15:05:23 | openstackgerrit | Merged openstack/os-vif stable/queens: add upper_constraints support https://review.opendev.org/658780 | |
| 15:05:39 | sean-k-mooney | just on a meeting but will look | |
| 15:05:44 | sean-k-mooney | after | |
| 15:07:43 | sean-k-mooney | mriedem: mock open is python3 only i think | |
| 15:08:18 | sean-k-mooney | there was a reason we dont use it in os-vif i think nova has implemented a fallback for py27 | |
| 15:13:16 | mriedem | we get mock in py27 from the mock library | |
| 15:13:22 | ralonsoh | mriedem, but this is what I'm doing (same as the example provided in the link): I'm replacing open with mock_open | |
| 15:13:53 | sean-k-mooney | mriedem: we dont in os-vif i think | |
| 15:14:02 | sean-k-mooney | e.g. we are not using the lib version | |
| 15:14:04 | sean-k-mooney | we could | |
| 15:14:23 | ralonsoh | mriedem, with "@mock.patch.object(builtins, 'open')" I'm avoiding the problem between py2 and py3 | |
| 15:14:34 | ralonsoh | mriedem, sean-k-mooney mock_open is in both versions | |
| 15:14:48 | mriedem | sean-k-mooney: umm, i don't see how py27 tests in os-vif could work then | |
| 15:14:52 | sean-k-mooney | ralonsoh: i dont think its in py27 without the mock lib | |
| 15:14:58 | mriedem | os-vif isn't directly requiring mock but it's getting it from oslotest https://github.com/openstack/oslotest/blob/master/requirements.txt | |
| 15:15:02 | mriedem | as a transitive dependency | |
| 15:15:07 | aspiers | sean-k-mooney: we use mock_open in lots of places | |
| 15:15:08 | sean-k-mooney | ah ok | |
| 15:15:24 | aspiers | see also https://review.opendev.org/#/c/661266/ although I don't think it's useful in this case | |
| 15:15:25 | sean-k-mooney | in which case then yes we could replace all of those with mock_open | |
| 15:16:43 | sean-k-mooney | aspiers: maybe im still not conviced that is better but that is becasue at least in os-vif i dont think we need it for our current tests | |
| 15:16:58 | sean-k-mooney | but yes it might be useful | |
| 15:17:02 | aspiers | BTW mock_open does not work in all scenarios | |
| 15:17:32 | aspiers | e.g. if the code reading the file uses enumerate() | |
| 15:17:35 | sean-k-mooney | aspiers: yes i have noticed it didnt work when i have tried to used it somethimes | |
| 15:17:48 | aspiers | see my comment May 24 9:18PM on https://review.opendev.org/#/c/661266/2 | |
| 15:18:11 | aspiers | e.g. it does not work when mocking oslo.config files | |
| 15:21:59 | openstackgerrit | Kashyap Chamarthy proposed openstack/nova master: libvirt: Rework 'EBUSY' (SIGKILL) error handling code path https://review.opendev.org/639091 | |
| 15:22:19 | kashyap | aspiers: Can I get my ACK, now? :D ---^ | |
| 15:22:23 | aspiers | probably | |
| 15:23:05 | kashyap | aspiers: On addressing the NOTE thing, indeed what MattB said (and what my note itself says) | |
| 15:23:16 | aspiers | kashyap: I probably misread it | |
| 15:23:33 | kashyap | aspiers: Aw, thought I write clearest phrases | |
| 15:23:55 | aspiers | "TODO(kchamart): Rewrite the above note, ..." | |
| 15:24:02 | openstackgerrit | Stephen Finucane proposed openstack/nova master: docs: Don't version links to reno docs https://review.opendev.org/661967 | |
| 15:24:08 | kashyap | aspiers: (I.e. the TODO will be addresed _once_ the MIN_LIBVIRT_VERSION reaches v4.7.0) | |
| 15:24:19 | aspiers | Yes, but you buried that qualifier deep in the sentence :) | |
| 15:24:24 | stephenfin | efried, mriedem: Added a bug link to https://review.opendev.org/#/c/661967/ | |
| 15:24:40 | aspiers | kashyap: "TODO(kchamart): Once MIN_LIBVIRT_VERSION reaches v4.7.0, rewrite the above note ..." | |
| 15:24:43 | kashyap | aspiers: Yeah, darn. I always worry sometimes, whether the qualifier is better at the start or a bit deeper inside | |
| 15:24:55 | aspiers | At the start, when it modifies the entire context | |
| 15:25:13 | kashyap | aspiers: Fair enough, actually let me do that. It will satisfiy my OCD-ness | |
| 15:25:17 | aspiers | :) | |
| 15:25:20 | aspiers | mine too | |
| 15:25:56 | kashyap | I even vaguely recall starting with the qualifier, but not sure what crack I was smoking, and changed it | |
| 15:26:06 | aspiers | No worries :) | |
| 15:27:34 | aspiers | kashyap: Once 4.7.0 as min version is reached, it's not just the note which gets rewritten, right? Indeed, I think you already say this but the comma after "above note" slightly obscures that the code needs to be removed too. | |
| 15:28:09 | aspiers | When it's reached, it's at least as important to update the code as the note, so I would write it more like: | |
| 15:28:14 | kashyap | aspiers: No, not just the note; the "remove the following code" as well | |
| 15:28:20 | kashyap | New phrasing coming soon to an IRC window near you | |
| 15:28:26 | aspiers | kashyap: "TODO(kchamart): Once MIN_LIBVIRT_VERSION reaches v4.7.0, remove the code [...] and rewrite the above note ..." | |
| 15:28:36 | aspiers | i.e. put the code removal first | |
| 15:29:05 | aspiers | Unfortunately my pedantry has no "off" button | |
| 15:31:50 | kashyap | I enjoy English phrasing pedantry. | |
| 15:32:16 | aspiers | Me too, for some reason | |
| 15:34:01 | kashyap | aspiers: How about this: | |
| 15:34:01 | kashyap | # TODO(kchamart): Once MIN_LIBVIRT_VERSION | |
| 15:34:02 | kashyap | # reaches v4.7.0, (a) rewrite the above note, | |
| 15:34:02 | kashyap | # and (b) remove the following code that retries | |
| 15:34:02 | kashyap | # _destroy() API call (which gives SIGKILL 30 | |
| 15:34:04 | kashyap | # seconds to take effect) -- because from v4.7.0 | |
| 15:34:06 | kashyap | # onwards, libvirt _automatically_ increases the | |
| 15:34:10 | kashyap | # timeout to 30 seconds. This was added in the | |
| 15:34:11 | kashyap | # following libvirt commits: | |
| 15:34:22 | aspiers | Yeah, that's good | |
| 15:34:35 | mriedem | sean-k-mooney: i bet you know the answer to this, is "x in y" where y is a set the same time as if y were a dict? | |
| 15:34:39 | aspiers | the (a) and (b) addresses the problem with the comma after "note" | |
| 15:34:56 | kashyap | aspiers: Yeah, indeed, it can't be misunderstood even if one tries to. | |
| 15:35:24 | sean-k-mooney | if y is a dict its checking is x in the keys | |
| 15:35:25 | aspiers | mriedem: same time as in complexity? or same thing? I think the answer to both is yes | |
| 15:35:38 | aspiers | yeah, what sean-k-mooney said | |
| 15:35:55 | sean-k-mooney | mriedem: if you want to check the values you need to do x in y.values | |
| 15:35:57 | mriedem | sean-k-mooney: yes i realize what it's looking for | |
| 15:35:57 | aspiers | both should be O(1) | |
| 15:36:04 | mriedem | i'm asking if both are O(1) | |
| 15:36:19 | sean-k-mooney | yes they are both O(1) | |
| 15:37:38 | openstackgerrit | Kashyap Chamarthy proposed openstack/nova master: libvirt: Rework 'EBUSY' (SIGKILL) error handling code path https://review.opendev.org/639091 | |
| 15:38:15 | kashyap | mdbooth: aspiers ^ Wording in the note rephrased to clearly spell out what's to do. | |
| 15:40:04 | aspiers | kashyap: is https://review.opendev.org/#/c/655193/ next on your list? since that's a major blocker for the newer changes on my SEV series | |
| 15:40:21 | kashyap | aspiers: Without guessing, is it the 'os_traits' rework? Yes | |
| 15:40:32 | aspiers | kashyap: and it sounds like pretty much all you have to do is empty x86.py | |
| 15:40:35 | aspiers | OK, cool | |
| 15:40:35 | kashyap | aspiers: Tomm is holiday; but today & Friday I'll be working on it, too. | |
| 15:40:41 | kashyap | aspiers: Yeah, let me do that right away | |
| 15:42:00 | kashyap | aspiers: It's amazing, what that change turned into. I was just a drive-by "observer" of the 'os_traits' repo & the corresponding call in driver.py. Thought I'll make a tiny change | |
| 15:42:04 | aspiers | kashyap: oh, and some simple nits from me in amd.py | |
| 15:42:07 | kashyap | ... and it turned into a multi-week thing | |
| 15:42:20 | kashyap | aspiers: Certainly, saw that. Typo: s/SVM/SEV/ | |
| 15:42:32 | kashyap | And others. Reading... | |