| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-05-29 | |||
| 14:03:33 | mriedem | bauzas: can you take a look at this functional regression recreate test patch? https://review.opendev.org/#/c/661822/ | |
| 14:03:38 | mriedem | related to your favorite, request spec | |
| 14:17:25 | stephenfin | mriedem: You think we can push ahead with this now? https://review.opendev.org/#/c/629597/ | |
| 14:20:19 | stephenfin | efried: Easy docs fix here that'll need to be backported after https://review.opendev.org/661967 | |
| 14:22:28 | efried | stephenfin: +2 | |
| 14:23:33 | mriedem | stephenfin: you will have to decouple it from https://review.opendev.org/#/c/620111/ | |
| 14:24:16 | mriedem | which is waiting on how https://review.opendev.org/#/c/637217/ gets sorted out which i'm not actively working on | |
| 14:24:46 | stephenfin | okidok | |
| 14:33:55 | sean-k-mooney | mriedem: johnthetubaguy when one of ye have time can you look at this backport for os-vif | |
| 14:34:15 | sean-k-mooney | we dont use upper constraits on all stable branches so we want to fix that | |
| 14:34:34 | sean-k-mooney | it still need more backports but that one is blocking a different backport too | |
| 14:39:52 | mriedem | stephenfin: do you plan on backporting this? https://review.opendev.org/#/c/661967/ | |
| 14:40:04 | mriedem | sean-k-mooney: did you intend on pasting a link to a backport? | |
| 14:40:12 | stephenfin | mriedem: yeah, once it's merged | |
| 14:40:19 | mriedem | stephenfin: ok, then you should have a bug for it | |
| 14:40:44 | stephenfin | oh yeah, good point that. I can do that now | |
| 14:41:11 | sean-k-mooney | mriedem: yes https://review.opendev.org/#/c/658780/ is the queens backport | |
| 14:41:44 | sean-k-mooney | mriedem: of using upper constaints | |
| 14:42:22 | sean-k-mooney | there is also the backport of https://review.opendev.org/#/q/topic:bug/1825888+(status:open+OR+status:merged) but that needs to be done on stien and rocky first | |
| 14:50:17 | mriedem | sean-k-mooney: https://review.opendev.org/#/c/658780/ looks like it's doing the trick but i can't tell at all from the tox logs, a comment in line if you have the answer | |
| 14:50:35 | mriedem | http://logs.openstack.org/80/658780/4/check/openstack-tox-py35/b70c01b/tox/py35-1.log | |
| 14:50:35 | mriedem | the tox logs make it look like they are installing from master u-c: | |
| 14:50:42 | mriedem | cmd: /home/zuul/src/opendev.org/openstack/os-vif/.tox/py35/bin/pip install -U -c/home/zuul/src/opendev.org/openstack/requirements/upper-constraints.txt -r/home/zuul/src/opendev.org/openstack/os-vif/requirements.txt -r/home/zuul/src/opendev.org/openstack/os-vif/test-requirements.txt | |
| 14:50:52 | mriedem | Collecting oslo.versionedobjects===1.31.3 (from -c /home/zuul/src/opendev.org/openstack/requirements/upper-constraints.txt (line 235)) Downloading http://mirror.bhs1.ovh.openstack.org/pypifiles/packages/fc/55/09c89a3c1724732f435b1639640ac58f4ab485f07c26aa5d1e452633040f/oslo.versionedobjects-1.31.3-py2.py3-none-any.whl (74kB) | |
| 14:51:56 | sean-k-mooney | hum ill take a closer look again. the tox job should overdie the uppercontarints file use to use teh one it cloned by setting the env varible | |
| 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 | # TODO(kchamart): Once MIN_LIBVIRT_VERSION | |
| 15:34:01 | kashyap | aspiers: How about this: | |
| 15:34:02 | kashyap | # _destroy() API call (which gives SIGKILL 30 | |
| 15:34:02 | kashyap | # and (b) remove the following code that retries | |
| 15:34:02 | kashyap | # reaches v4.7.0, (a) rewrite the above note, | |
| 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: | |