| opendevreview | Merged opendev/system-config master: Update gerrit image to v3.2.11 https://review.opendev.org/c/opendev/system-config/+/799225 | 00:08 |
|---|---|---|
| opendevreview | Merged opendev/system-config master: Good riddance to track-upstream and its cronjob https://review.opendev.org/c/opendev/system-config/+/799124 | 00:08 |
| opendevreview | Merged opendev/system-config master: Stop updating Gerrit RDBMS for repo renames https://review.opendev.org/c/opendev/system-config/+/797990 | 00:08 |
| opendevreview | Ian Wienand proposed openstack/diskimage-builder master: fedora-container: install dnf-plugins-core https://review.opendev.org/c/openstack/diskimage-builder/+/796984 | 01:03 |
| opendevreview | Ian Wienand proposed opendev/system-config master: certcheck: also check linaro API https://review.opendev.org/c/opendev/system-config/+/799731 | 02:40 |
| ianw | linaro seems happy, but we've go osuosl images stuck again | 04:11 |
| opendevreview | Merged openstack/diskimage-builder master: Do not uninstall non-installed packages https://review.opendev.org/c/openstack/diskimage-builder/+/793097 | 04:55 |
| opendevreview | Ian Wienand proposed opendev/zone-opendev.org master: Add paste01 https://review.opendev.org/c/opendev/zone-opendev.org/+/799735 | 05:11 |
| opendevreview | Ian Wienand proposed opendev/system-config master: Add paste service https://review.opendev.org/c/opendev/system-config/+/798400 | 05:12 |
| opendevreview | Ian Wienand proposed opendev/system-config master: lodgeit: use mariadb connector https://review.opendev.org/c/opendev/system-config/+/799004 | 05:12 |
| opendevreview | Ian Wienand proposed opendev/system-config master: Add opendev paste server https://review.opendev.org/c/opendev/system-config/+/799736 | 05:12 |
| *** iury|holiday is now known as iurygregory | 05:53 | |
| opendevreview | Merged openstack/diskimage-builder master: fedora-container: install dnf-plugins-core https://review.opendev.org/c/openstack/diskimage-builder/+/796984 | 06:51 |
| *** amoralej|off is now known as amoralej | 07:08 | |
| *** jpena|off is now known as jpena | 07:46 | |
| *** ysandeep is now known as ysandeep|lunch | 07:46 | |
| *** ykarel is now known as ykarel|lunch | 08:32 | |
| *** bhagyashris_ is now known as bhagyashris|ruck | 08:37 | |
| *** whoami-rajat is now known as Guest89 | 09:05 | |
| opendevreview | Merged opendev/base-jobs master: Fix docstrings to match job updates https://review.opendev.org/c/opendev/base-jobs/+/799720 | 09:09 |
| *** ysandeep|lunch is now known as ysandeep | 09:27 | |
| *** rpittau|afk is now known as rpittau | 09:42 | |
| *** ykarel|lunch is now known as ykarel | 10:14 | |
| *** jpena is now known as jpena|lunch | 11:39 | |
| *** ysandeep is now known as ysandeep|afk | 12:31 | |
| opendevreview | Merged opendev/system-config master: certcheck: also check linaro API https://review.opendev.org/c/opendev/system-config/+/799731 | 12:33 |
| *** amoralej is now known as amoralej|lunch | 12:38 | |
| opendevreview | Dmitriy Rabotyagov proposed openstack/project-config master: Deprecate os-panko role https://review.opendev.org/c/openstack/project-config/+/799802 | 12:42 |
| *** jpena|lunch is now known as jpena | 12:43 | |
| opendevreview | Dmitriy Rabotyagov proposed openstack/project-config master: Remove noop jobs for deprecated os-panko https://review.opendev.org/c/openstack/project-config/+/799808 | 12:46 |
| *** ysandeep|afk is now known as ysandeep | 13:17 | |
| opendevreview | Dmitriy Rabotyagov proposed openstack/project-config master: Create repo for Hashicorp Vault deployment https://review.opendev.org/c/openstack/project-config/+/799822 | 13:18 |
| opendevreview | Dmitriy Rabotyagov proposed openstack/project-config master: Add Vault role to zuul config https://review.opendev.org/c/openstack/project-config/+/799824 | 13:22 |
| opendevreview | Dmitriy Rabotyagov proposed openstack/project-config master: Add Vault role to Zuul jobs https://review.opendev.org/c/openstack/project-config/+/799825 | 13:22 |
| *** amoralej|lunch is now known as amoralej | 13:36 | |
| *** bhagyashris is now known as bhagyashris|ruck | 13:59 | |
| *** bhagyashris_ is now known as bhagyashris|ruck | 14:24 | |
| *** ykarel is now known as ykarel|away | 15:04 | |
| *** ysandeep is now known as ysandeep|dinner | 15:12 | |
| opendevreview | wes hayutin proposed openstack/project-config master: update channel config with new registered nic https://review.opendev.org/c/openstack/project-config/+/799855 | 15:34 |
| opendevreview | Jeremy Stanley proposed opendev/git-review master: Ignore unstaged/uncommitted submodule changes https://review.opendev.org/c/opendev/git-review/+/799861 | 16:01 |
| fungi | that's wip to demonstrate job failures before i push a revision with the associated fix | 16:01 |
| *** marios is now known as marios|out | 16:06 | |
| *** rpittau is now known as rpittau|afk | 16:09 | |
| clarkb | Anyone know who is runnign stackalytics these days? Seems there may even be two of them with different teams involved? One of the accounts that has external id conflicts appears related to stackalytics and I worry that the bot email addrs won't end up in front of humans | 16:19 |
| JayF | stackalytics.io is updated, stackalytics.com is/was not updated for a long time | 16:20 |
| JayF | I don't know who runs either one, whcih probably makes that not a helpful answer | 16:20 |
| *** ysandeep|dinner is now known as ysandeep | 16:22 | |
| clarkb | ya in this case I'm fairly certain the accounts are for .com based on their age | 16:24 |
| opendevreview | Jeremy Stanley proposed opendev/git-review master: Ignore unstaged/uncommitted submodule changes https://review.opendev.org/c/opendev/git-review/+/799861 | 16:27 |
| fungi | and now that revision should succeed all its jobs | 16:27 |
| *** amoralej is now known as amoralej|off | 16:30 | |
| fungi | according to a recent rackspace support ticket, nl04 got rebooted yesterday due to a hypervisor host issue | 16:49 |
| fungi | looks like it's currently running though | 16:50 |
| *** jpena is now known as jpena|off | 16:52 | |
| *** ysandeep is now known as ysandeep|away | 16:53 | |
| *** mgoddard- is now known as mgoddard | 16:54 | |
| clarkb | fungi: so it is safe to rebase in the parent repo when the submodules are dirty? | 16:58 |
| clarkb | autostash won't discard changes in the submodule? | 16:58 |
| fungi | seems to be | 16:58 |
| fungi | rebasing in the superrepo shouldn't change the state of the submodules anyway (i did not test whether autostash stashes the submodule worktrees but it seems highly unlikely since the rebase wouldn't operate on them to begin with) | 16:59 |
| clarkb | fungi: ok, I left a note on the change to assert the submodule hasn't changed after running git review | 17:00 |
| clarkb | we don't seem to check that in the tests and that seems important given the regression | 17:01 |
| fungi | well, the test rebase wouldn't alter anything, it does a reset after, the problem arises when rebase.autostash is set to stash things before rebasing, since the reset causes the stash to be dropped | 17:02 |
| clarkb | fungi: will the reset change the usbmodule though? I think you have to do that separately | 17:04 |
| clarkb | but that may not be necessary at all since the submodule will hopefully be ignored when rebasing the parent. | 17:05 |
| clarkb | But I think we should actualyl check the state of the submodule if that is what we are worried about here | 17:05 |
| fungi | could just as easily ask whether the rebase will change the submodule in that case | 17:05 |
| clarkb | (it isn't enough to know git review can push successfully in this case) | 17:05 |
| fungi | autostash should only stash what's being rebased, and a rebase of the superrepo shouldn't rebase submodules | 17:06 |
| fungi | any more than the reset of the superrepo would reset the submodules | 17:07 |
| clarkb | right I understand that, but to properly test the change being made we need to check that I think. Otherwise we could be reintroducing the previous regression to submodule users | 17:07 |
| fungi | but would that be a git-review bug or a git bug at that point? | 17:07 |
| clarkb | it would be a git issue, but considering how painful submodules are to use I doubt we'd get much sympathy and just be told to deal with it. | 17:08 |
| clarkb | submodules are known for these types of gotchas | 17:08 |
| fungi | well, my point is, how do we detect whether git rebased the submodule and then reset it? the end result will be the same as if it was not rebased and not reset | 17:09 |
| clarkb | I think we check if the end result is not the same as that would be what users notice | 17:09 |
| fungi | users only noticed it for normal repos if they were using rebase.autostash, which we don't (currently) try to turn on in our tests | 17:10 |
| fungi | seems like we would need tests explicitly enabling rebase.autostash to detect that case | 17:11 |
| clarkb | oh we didn't add tests for autostash when we fixed the main issue? | 17:11 |
| fungi | no, we simply made git-review break when there are unstaged or uncommitted changes | 17:12 |
| clarkb | I guess we just short circuited before running at all but now we want to run in some cases which makes it more complicated to test | 17:12 |
| fungi | so the newly proposed fix rescopes the prior fix to only apply on unstaged or uncommitted changes in the superrepo and ignore submodules, as the rebase should never touch them to begin with | 17:13 |
| fungi | before the original rebase.autostash fix, the git-review behavior was to catch unstaged or uncommitted changes at push and let it give a more opaque error, at which point it had already discarded the unwound stash from rebase.autostash users | 17:14 |
| fungi | we fixed that by detecting unstaged and uncommitted changes earlier and giving a clearer error message | 17:15 |
| fungi | but we missed that the solution used to detect those did not ignore submodules (like the rebase/reset did, as evidenced by this being a regression for users with dirty submodules) | 17:16 |
| clarkb | ya I think my concern is that git interacts with submodules in unexpected ways. For example why does git diff in the parent show you the delta for submodules? But then apparently reabse ignores submodules unless you pass it extra flags. I just worry that auto stash or rebase will do the unexpected here and then we regress those users again | 17:17 |
| clarkb | rather than telling them to clean up their submoduels first which is the current state right? | 17:17 |
| clarkb | I'm not entirely convinced being told to clean up your submoduels first is the worst thing | 17:17 |
| clarkb | fungi: hrm I'm not able to reproduce the dirty submodule issue either fwiw | 17:19 |
| fungi | the regression tests demonstrate it on patchset 1 of 799861 | 17:20 |
| fungi | you can see i the logs that git diff doesn't report a clean result when there are unstaged or uncommitted changes in the added submodule | 17:21 |
| clarkb | ok what I get is +Subproject commit 73f8acdc5c97e068143c86765995c4fb6923ee91-dirty | 17:21 |
| clarkb | I guess it doesn't really diff the submodule but does show you the submodule is dirty | 17:21 |
| fungi | right | 17:21 |
| fungi | and we're basically just relying on git diff to report there's no change | 17:21 |
| clarkb | Right if we land your change then git-review users will be able to have dirty submodules and push their clean parent repos up. I'm trying to think over if that is actually correct in my head | 17:22 |
| clarkb | if the parent changes rely on the dirty submodule updates to function then the most correct thing would be for the user to correct that. | 17:23 |
| clarkb | However in this case git-review is only interested in whether or not the parent changes are mergeable | 17:23 |
| clarkb | (it doesn't care about correctness or CI results beyond is this able to merge when it hits the gerrit server) | 17:23 |
| fungi | prior to the regression, git-review users were able to push changes when dirty submodules were present (and not when the superrepo worktree was dirty), so this regains the original behavior in that regard | 17:24 |
| clarkb | yes, and having a dirty submodule isn't a problem when it comes to mergeability. It would be what ever hash for the submodule you have committed against whatever is upstream. If upstream has changed and downstream has changed then you will need to rebase downstream to address that and I think that would be detected in your chagne still | 17:25 |
| fungi | the rebase.autostash fix didn't really change whether users could push changes with a dirty worktree (that didn't work before it either), just where in the process we detected the case. the fix there was to bail sooner rather than letting it break at push | 17:25 |
| fungi | but in so doing, we made it impossible to push when there was a dirty submodule present | 17:26 |
| clarkb | right and my first impression is that that is a good thing :) but I think it doesn't really matter much to git-review after thinking that through a bit more | 17:26 |
| fungi | which used to work | 17:26 |
| clarkb | personally for my own use of git I would like git-review to not let me push code if I have dirty submodules because that would likely indicate I have uncommitted changes necessary to make my change work | 17:27 |
| clarkb | But git-review as a tool doesn't really care about that, just that it can check if the resulting push is mergeable | 17:27 |
| clarkb | and we regressed on making that check accurate | 17:27 |
| fungi | we could certainly add an option to break on dirty submodules, but that would be a new feature since the current behavior there is an unintended change from the prior release | 17:28 |
| clarkb | its also complicated because what you likely want to do si push/git-review the submodule first. Then once you know its commit hash update the child | 17:30 |
| clarkb | gerrit sort of works around this with the auto updating submoduesl though | 17:30 |
| *** gthiemon1e is now known as gthiemonge | 20:57 | |
| opendevreview | wes hayutin proposed opendev/elastic-recheck master: Run elastic-recheck container https://review.opendev.org/c/opendev/elastic-recheck/+/729623 | 21:24 |
| clarkb | weshay|ruck: I don't have time to really help with ^ or look into it in depth, but one of the things that e-r has always struggled with is coupling too much of the operational information and the query list with the software itself. You might want to have a more generic dockerfile in the repo that can be reconsumed or FROM'd downstream to avoid making that coupling worse | 21:35 |
| weshay|ruck | clarkb, you are very responsive... :) thanks... to be clear.. I'm really only expecting some folks in tripleo to look at this with me. I'm just dipping my toes in now | 21:37 |
| weshay|ruck | I'll take this input and run it by the folks tomorrow | 21:38 |
| clarkb | sure, I just always saw the queries being vendored as a bug in e-r and that change looks very site specific too. I know the existing coupling of various concerns in that repo probalby makes this messier too | 21:38 |
| weshay|ruck | clarkb, I totally understand infra not having time for this thing.. | 21:38 |
| clarkb | weshay|ruck: also it might make sense to break the chagnes down into things like python3 compat, relative url paths, dockerfile, etc | 21:38 |
| clarkb | just to make it easier to understand the various bits going on | 21:39 |
| weshay|ruck | hrm k.. ya.. I need to get my own hands dirty here a little bit more to have anything very intelligent to say at this point | 21:39 |
| weshay|ruck | k | 21:39 |
| weshay|ruck | clarkb, appreciate the advice! | 21:40 |
Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!