| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-05-28 | |||
| 17:25:20 | efried | aspiers: I'm pretty confused about the rebasing state of all your patches. | |
| 17:25:32 | aspiers | efried: how can I help with that? | |
| 17:25:45 | efried | are you working on rebasing all of them to be in one chain? | |
| 17:25:57 | aspiers | only where necessary | |
| 17:26:38 | aspiers | my goals, in decreasing order, are: 1) correctness, 2) ease of review, 3) minimise churn on CI resources | |
| 17:27:33 | aspiers | it did recently occur to me that I could (theoretically, in a parallel universe where time is unconstrained) teach git-review to automatically rebase as necessary by using my git-deps module | |
| 17:29:12 | efried | what I mean is, for example, https://review.opendev.org/#/c/661782/2 is based on https://review.opendev.org/#/c/661266/3 when PS4 is available (and merging). And then https://review.opendev.org/#/c/638680/18 is based on https://review.opendev.org/#/c/661782/2, but https://review.opendev.org/#/c/636334/5 is based on https://review.opendev.org/#/c/638680/14 | |
| 17:30:32 | aspiers | efried: that's because I forgot that you submitted PS4 | |
| 17:30:50 | aspiers | efried: I just based it on the latest PS I had locally, without thinking | |
| 17:30:52 | efried | okay, my fault on that one | |
| 17:30:59 | aspiers | haha, hardly :) | |
| 17:31:18 | aspiers | but I'll generously share some of the blame with you if you insist ;) | |
| 17:31:37 | efried | anyway, as a reviewer I'm basically too confused to jump in atm | |
| 17:31:49 | aspiers | looking at the other ones now | |
| 17:31:50 | efried | which is fine, I have lots to keep me busy, not like I'm sitting on my hands waiting for this to be ready. | |
| 17:32:15 | efried | just want you to be aware that I'm likely to wait until there's no orange dots or green tildes on the right before I dig in. | |
| 17:32:25 | spatel | sean-k-mooney: hey | |
| 17:32:38 | efried | aspiers: (are you aware of the meaning of the orange dots and green tildes?) | |
| 17:32:41 | aspiers | OK, so essentially you want me to ditch my third goal? which is totally fine by me | |
| 17:32:46 | aspiers | yep, I had figured that out | |
| 17:33:41 | aspiers | I can understand the desire to eliminate orange dots. Why do you want to avoid green tildes though? | |
| 17:34:03 | efried | aspiers: well, I may be telling you that you may be reversing goals 2 and 3 :) at least from my pov | |
| 17:34:27 | aspiers | ACK, just trying to understand the cause of the confusion | |
| 17:34:29 | efried | because it means that those patches are based on older versions of these | |
| 17:34:43 | efried | it's usually a signal that you're still in the process of rebasing and cleaning things up. | |
| 17:34:56 | aspiers | ... which in the third example you listed is true | |
| 17:35:10 | efried | which is reinforced by having just WIP'd one near the bottom | |
| 17:35:20 | efried | anyway, again, I'm not trying to hurry you along. | |
| 17:35:29 | aspiers | I don't mind being hurried :) | |
| 17:35:56 | efried | just wanted to confirm that you're still shuffling the paperwork, as opposed to thinking you're ready for reviews and then getting frustrated when they don't happen | |
| 17:36:21 | aspiers | Well, this is a very useful discussion | |
| 17:37:18 | aspiers | It highlights that the unavoidable impact of deciding not to review anything within a given topic until all tildes and dots are gone, is that every time I need to change something minor lower in the dependency tree, I have to rebase every single thing above it before the new patchset gets reviewed | |
| 17:37:40 | aspiers | To me this highlights a failure in Gerrit's UI | |
| 17:38:17 | efried | or a failure in my method of working. | |
| 17:38:20 | aspiers | Ideally reviews would only be blocked if stale stuff is *below* them, not above | |
| 17:38:28 | efried | "blocked" is a bit strong | |
| 17:39:11 | efried | It's just the typical workflow I see (and use) is, when something needs to be fixed on a lower patch, download the whole series, git restack, fix the issues below, which automatically rebases the rest of the way up, and resubmit the whole series. | |
| 17:39:17 | efried | Agree that consumes a lot of CI resources | |
| 17:39:37 | efried | but it also ensures that the top changes will legitimately work with however the lower changes have been altered. | |
| 17:39:48 | aspiers | Sure | |
| 17:39:57 | aspiers | I'm not saying I want to avoid rebasing things | |
| 17:40:25 | aspiers | Just wondering if the presence of WIP higher up the tree should have an impact on stuff lower in the tree | |
| 17:41:16 | aspiers | If I understand the Gerrit UI correctly, we're talking about whether a patch in the Related Changes list has dots or tildes *below* it, right? | |
| 17:42:07 | efried | I'm not sure it's possible to have tildes below or dots above. | |
| 17:42:29 | efried | ...unless you're looking at a PS that itself has a dot | |
| 17:42:29 | efried | I mean, technically possible, but you would really have to be trying. | |
| 17:42:29 | aspiers | Oh, good point | |
| 17:42:30 | efried | anyway | |
| 17:42:57 | efried | the presence of WIP higher in the tree (which is not the same thing as a tilde, btw) should not impact reviews lower. | |
| 17:43:21 | efried | so this is probably just me being used to one workflow and you using a different one to (rightly) try to conserve CI resources. | |
| 17:44:18 | aspiers | Well, it's also about me needing to pay more attention to orange dots in Related Changes, so I can spot when rebases are needed | |
| 17:44:20 | efried | moot right now because I've reviewed everything up to the first one that's actually titled WIP, so regardless of colored decorations, I'm stopping here. | |
| 17:44:52 | aspiers | the WIP one is only marked WIP because of the required-projects hack. Other than that I think it's ready to go | |
| 17:44:56 | efried | but yeah, usually a patch that's riding an orange dot should be rebased if it's to get serious attention. | |
| 17:45:20 | aspiers | At least, other than rebasing (which I'll do now) I've run out of improvements to make to it | |
| 17:46:21 | aspiers | I put WIP on it because mriedem asked me to, but the only element of WIP there is that kashyap's os-traits patch hasn't landed and been released yet. | |
| 17:48:07 | aspiers | efried: also, since we're putting the workflow under the microscope, should I be putting the SEV blueprint: in the commit message and topic for changes like refactorings, which aren't per se SEV-related, but still form part of the SEV series? | |
| 17:48:39 | efried | nah | |
| 17:48:43 | aspiers | ok | |
| 17:50:37 | openstackgerrit | Adam Spiers proposed openstack/nova master: extract baselineCPU API call from _get_cpu_traits() https://review.opendev.org/661782 | |
| 17:53:59 | openstackgerrit | Adam Spiers proposed openstack/nova master: WIP: Provide HW_CPU_X86_AMD_SEV trait when SEV is supported https://review.opendev.org/638680 | |
| 17:58:34 | openstackgerrit | Merged openstack/nova-specs master: Update SEV work item to new approach based on MEM_ENCRYPTION_CONTEXT https://review.opendev.org/655717 | |
| 17:59:08 | openstackgerrit | Adam Spiers proposed openstack/nova master: Extract SEV-specific bits on host detection https://review.opendev.org/636334 | |
| 18:01:18 | aspiers | efried: got rid of some orange dots | |
| 18:05:09 | aspiers | (and that's from someone who's worked with them) | |
| 18:16:59 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Workaround missing RequestSpec.instance_group.uuid https://review.opendev.org/661786 | |
| 18:17:00 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add regression recreate test for bug 1830747 https://review.opendev.org/661822 | |
| 18:17:01 | mriedem | zigo: with a functional recreate test of the issue ^ | |
| 18:17:01 | openstack | bug 1830747 in OpenStack Compute (nova) "Error 500 trying to migrate an instance after wrong request_spec" [High,In progress] https://launchpad.net/bugs/1830747 - Assigned to Matt Riedemann (mriedem) | |
| 18:24:42 | openstackgerrit | Adam Spiers proposed openstack/nova master: Reduce logging of host hypervisor capabilities to DEBUG level https://review.opendev.org/661379 | |
| 18:34:09 | ganso | lyarwood, melwitt, dansmith, tonyb: Hello! When you have a minute, could you please take a look at those backports? https://review.opendev.org/#/c/652153 , https://review.opendev.org/#/c/658136 and https://review.opendev.org/#/c/661349 ? Thanks in advance! =) | |
| 18:44:14 | aspiers | efried: if CONF.libvirt.num_memory_encrypted_guests is not set and therefore defaults to None (meaning as we agreed in the spec, "auto-detect the inventory, if if this isn't possible, don't impose any limit), what should the MEM_ENCRYPTION_CONTEXT inventory get set to? | |
| 18:44:30 | aspiers | I'm not sure we came to a consensus with jaypipes on this http://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2019-04-23.log.html#t2019-04-23T17:55:57 | |
| 18:48:58 | efried | aspiers: MAXINT | |
| 18:49:10 | jaypipes | aspiers: not sure you need any consensus. :) | |
| 18:51:18 | aspiers | jaypipes: OK :) | |
| 19:57:19 | openstackgerrit | John Garbutt proposed openstack/nova master: WIP: Integrating with unified limits https://review.opendev.org/615180 | |
| 20:51:52 | mriedem | yikun: looks like we have an api-ref bug on the server group policy parameter type, can you confirm? https://bugs.launchpad.net/nova/+bug/1830800 | |
| 20:51:53 | openstack | Launchpad bug 1830800 in OpenStack Compute (nova) "Compute API in nova - server group "policy" field is a string rather than an object" [Medium,Triaged] | |
| 20:53:48 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: Run revert resize tests in nova-live-migration https://review.opendev.org/653498 | |
| 20:53:48 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: [WIP] Revert resize: wait for events according to hybrid plug https://review.opendev.org/644881 | |
| 20:53:49 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: [DNM] use iptables in nova-multinode https://review.opendev.org/660782 | |
| 22:45:15 | eandersson | What is the network cache used for on the computes? | |
| 22:45:43 | openstackgerrit | Adam Spiers proposed openstack/nova master: extract baselineCPU API call from _get_cpu_traits() https://review.opendev.org/661782 | |
| 22:45:57 | eandersson | We found that with a high number of computes, the whole heal instance info cache got a little too chatty on the default setting. | |
| 22:46:32 | aspiers | efried: at risk of highlighting my own incompetence, this is the kind of change where normally I'd be inclined to avoid a rebase of everything sitting on top: https://review.opendev.org/#/c/661782/3..4/nova/virt/libvirt/driver.py | |
| 22:47:00 | aspiers | efried: that said, I'm totally fine with rebasing if you'd prefer that | |
| 22:47:56 | aspiers | ah actually, since splitting out some other patches, there's only one patch sitting on top | |
| 22:48:02 | aspiers | so maybe I will anyway | |
| 22:49:37 | aspiers | hah, especially considering zuul absolutely hates that change sitting on top | |
| 22:49:49 | efried | aspiers: I respect your green-ness. And maybe you'll start a sea change in nova. All I'm saying is that all the series contributions I've seen in my (albeit limited) time here have been managed in a series-as-a-whole fashion. | |
| 22:50:39 | aspiers | efried: fine by me :) I maintain the belief that Gerrit should visualise the dependency tree in a way which would make this kind of stuff a no-brainer | |
| 22:50:57 | aspiers | but until it gets smarter, I'm happy to do whatever to make reviews easier | |
| 22:51:59 | efried | aspiers: I use the tree-shaped version of git log locally, do you? | |
| 22:52:14 | efried | To me, that sounds more or less like the visualization you're talking about... | |
| 22:52:17 | aspiers | I use gitk, but that shows the history, not the real dependency tree | |
| 22:52:26 | efried | ...and in that visualization, the way it's expressed is bang on. | |
| 22:52:54 | efried | i.e. you can't say that PS4 of X is based on PS2 of Y when it's in fact based on PS1 of Y. | |
| 22:53:05 | aspiers | I'm talking about semantic dependencies https://aspiers.github.io/nashville-git-automagic-april-2019/#/git-deps-concept | |
| 22:53:07 | efried | Because PS1 and PS2 of Y could be *wildly* bifferent. | |