Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-05-29
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...
15:44:53 aspiers kashyap: that's what happened to me with https://review.opendev.org/#/c/538498/, more or less
15:45:31 kashyap aspiers: Ah, I saw this fly by. Thanks for the diagrams!
15:46:08 aspiers kashyap: https://docs.openstack.org/nova/latest/reference/update-provider-tree.html#taxonomy-of-traits-and-capabilities
15:46:12 aspiers added since then
15:48:21 kashyap aspiers: (While I wait on the almost-human-sounding-bot), what's the source on that neat digram?
15:48:27 kashyap s/digram/diagram/
15:48:41 aspiers kashyap: it's just SVG
15:48:58 aspiers stephenfin: search and replace? ;-p
15:49:09 kashyap aspiers: Ah, okay.
15:49:11 stephenfin aspiers: Deleting cells v1
15:49:21 cdent that'll show you!
15:49:22 aspiers ROFL
15:49:47 stephenfin :D
15:50:07 aspiers WTF, I'm #4 by LoC
15:50:15 aspiers How is that possible
15:50:30 stephenfin specs?
15:50:36 cdent ugh, this is not great: https://www.stackalytics.com/?metric=loc&module=placement-group
15:51:48 edleafe cdent: they have been saying that their numbers have been out of whack for a while
15:52:04 cdent edleafe: they reported recently that they fixed it. that looks right
15:52:23 edleafe I thought they were still working on it
15:52:39 cdent there was a post couple days ago
15:52:50 cdent but who knows, I don't think we can ever really fully rely on it
15:52:51 openstackgerrit Stephen Finucane proposed openstack/nova stable/stein: docs: Don't version links to reno docs https://review.opendev.org/662025
15:52:53 melwitt mriedem, johnthetubaguy: thanks for the reviews on counting quotas <3 I'm going to add a follow up patch for the nits/notes and then also propose the spec amendment for the deviations from the spec
15:53:27 edleafe You mean Eric has only contribute 25 lines of code??
15:53:33 aspiers edleafe: they were missing a whole bunch of commits for nova, but I think that got fixed in the last day or 2
15:54:39 cdent edleafe: of merged stuff in train, yeah, I'd say that's right
15:54:40 aspiers edleafe: it says 809 LoC for Eric

Earlier   Later