| clarkb | corvus: I've approved the change. I worry the "naked" del project will be subject to future cleanups but I agree that removing the potnetial confusion is a good idea | 00:01 |
|---|---|---|
| corvus | oh yeah, i guess i could have added a comment | 00:12 |
| fungi | i did wonder about that. the commit message made the reason clear, but in retrospect a comment would have helped | 00:41 |
| opendevreview | Merged zuul/zuul master: Fix error with config cache some more https://review.opendev.org/c/zuul/zuul/+/805089 | 01:14 |
| ianw | it feels like https://review.opendev.org/804956 might be stuck; https://zuul.opendev.org/t/zuul/status shows it but it just seems to be sitting there | 02:06 |
| ianw | probably fallout of the restart but not sure | 02:06 |
| *** rpittau|afk is now known as rpittau | 07:08 | |
| opendevreview | Benjamin Schanzel proposed zuul/zuul master: Add tenant name on NodeRequests for Nodepool https://review.opendev.org/c/zuul/zuul/+/788680 | 07:18 |
| *** jpena|off is now known as jpena | 07:35 | |
| opendevreview | Simon Westphahl proposed zuul/zuul master: Fix config cache cleanup with extra config paths https://review.opendev.org/c/zuul/zuul/+/805175 | 09:53 |
| opendevreview | Simon Westphahl proposed zuul/zuul master: Fix config cache cleanup with extra config paths https://review.opendev.org/c/zuul/zuul/+/805175 | 10:30 |
| swest | corvus: thanks for fixing the bug and adding the missing cache cleanup! I found a small issue when using extra config paths with multiple tenants. that should be fixed with 805175 | 10:38 |
| swest | fungi, clarkb, tobiash: ^ | 10:38 |
| opendevreview | Sorin Sbârnea proposed zuul/zuul-jobs master: Include podman installation with molecule https://review.opendev.org/c/zuul/zuul-jobs/+/803471 | 10:39 |
| *** dviroel|ruck|out is now known as dviroel|ruck | 11:08 | |
| *** jpena is now known as jpena|lunch | 11:31 | |
| *** jpena|lunch is now known as jpena | 12:31 | |
| corvus | swest: i have a question on that, can you check in gerrit? https://review.opendev.org/805175 | 13:18 |
| corvus | swest: i don't understand which test is faulty and why | 13:19 |
| swest | corvus: the additional test I added is not faulty, but if you want to reproduce the describe issue with the current master you might need to run it multiple times | 13:19 |
| swest | the order in which the tenant reconfig events are processed is not deterministic | 13:20 |
| corvus | swest: because the first change touches both tenants, ok | 13:21 |
| swest | corvus: let me know if I should clarify this in the commit message | 13:22 |
| corvus | swest: why isn't it deterministic? | 13:23 |
| swest | corvus: I haven't checked | 13:26 |
| corvus | okay, i'll try to figure that out; i don't like the idea of merging a test we know isn't reliable | 13:27 |
| swest | it's reliable in the current form, it's just not reliable for reproducing the issue with current master :) | 13:27 |
| corvus | swest: understood -- it still seems like something that would be good to improve; it could be the source of more false-negatives in the future | 13:30 |
| swest | corvus: agree | 13:30 |
| opendevreview | James E. Blair proposed zuul/zuul master: Fix config cache cleanup with extra config paths https://review.opendev.org/c/zuul/zuul/+/805175 | 13:43 |
| corvus | swest: found it ^ | 13:43 |
| swest | corvus: that was quick :D thanks! | 13:44 |
| fungi | aha, so there was a race between the events appearing and polling | 13:44 |
| corvus | fungi: well, more that the scheduler was already in the middle of its run loop when the event appeared; with this change it will always start at the beginning of the loop | 13:45 |
| fungi | got it | 13:45 |
| swest | corvus: I guess we can remove the note about the reproducer being unstable | 13:47 |
| corvus | oh ya | 13:47 |
| opendevreview | James E. Blair proposed zuul/zuul master: Fix config cache cleanup with extra config paths https://review.opendev.org/c/zuul/zuul/+/805175 | 13:47 |
| opendevreview | Matthieu Huin proposed zuul/zuul master: web UI: user login with OpenID Connect https://review.opendev.org/c/zuul/zuul/+/734082 | 14:26 |
| opendevreview | Matthieu Huin proposed zuul/zuul master: Add authentication-realm attribute to tenants https://review.opendev.org/c/zuul/zuul/+/735586 | 14:53 |
| opendevreview | Matthieu Huin proposed zuul/zuul master: web UI: allow a privileged user to dequeue a change https://review.opendev.org/c/zuul/zuul/+/734850 | 14:54 |
| opendevreview | Matthieu Huin proposed zuul/zuul master: web UI: allow a privileged user to re-enqueue a change https://review.opendev.org/c/zuul/zuul/+/736772 | 14:54 |
| opendevreview | Matthieu Huin proposed zuul/zuul master: Web UI: allow a privileged user to request autohold https://review.opendev.org/c/zuul/zuul/+/768115 | 14:55 |
| opendevreview | Matthieu Huin proposed zuul/zuul master: Web UI: add Autoholds, Autohold page https://review.opendev.org/c/zuul/zuul/+/768199 | 14:55 |
| opendevreview | Matthieu Huin proposed zuul/zuul master: REST API, Web UI: add pipelines' manager, triggers data in status https://review.opendev.org/c/zuul/zuul/+/736968 | 14:55 |
| opendevreview | Matthieu Huin proposed zuul/zuul master: web UI: allow a privileged user to promote a change https://review.opendev.org/c/zuul/zuul/+/781858 | 14:55 |
| opendevreview | Matthieu Huin proposed zuul/zuul master: Web UI: Add "Create Autohold Request" form, improve API error messages https://review.opendev.org/c/zuul/zuul/+/802559 | 14:55 |
| opendevreview | Matthieu Huin proposed zuul/zuul master: Example Docker compose: keycloak integration https://review.opendev.org/c/zuul/zuul/+/769943 | 14:55 |
| *** y2kenny is now known as Guest4828 | 15:23 | |
| opendevreview | Matthieu Huin proposed zuul/zuul master: Web UI: Add "Create Autohold Request" form, improve API error messages https://review.opendev.org/c/zuul/zuul/+/802559 | 15:31 |
| clarkb | corvus: did you see ianw's question about 804956,2 not running jobs above? it is a zuul change in the zuul tenant just hanging out | 15:36 |
| corvus | missed that, i'll check | 15:39 |
| *** jpena is now known as jpena|off | 15:41 | |
| clarkb | related the stack at https://review.opendev.org/c/zuul/zuul/+/804602 has been updated. I don't like that last change, but the first two seem fine. | 15:43 |
| corvus | the merge request seems to have disappeared | 15:44 |
| corvus | 2021-08-18 23:51:51,663 DEBUG zuul.MergerApi: [e: cbc3fadbb60f4369b29a9f438c0f3c4b] Submitting job request to ZooKeeper <MergeRequest 25fd3a244498410ead1983358bb0d593, job_type=merge, state=requested, path=/zuul/merger/requests/25fd3a244498410ead1983358bb0d593> | 15:45 |
| corvus | that's the last we see of it | 15:45 |
| corvus | (unrelated - we seem to be leaking locks) | 15:46 |
| clarkb | interesting, how does the timing for that line up with when we restarted and reconfigured zuul? I think this was late enough to be afterwards? | 15:46 |
| corvus | yeah, it happened after the full-reconfigure was complete | 15:46 |
| clarkb | ya 23:33 is when I reported jobs had started after the restart and full reconfigure | 15:46 |
| corvus | about 20 minutes after | 15:46 |
| clarkb | unlikely to be an interaction between those events then | 15:47 |
| corvus | no tracebacks around the submission | 15:48 |
| corvus | and the cleanup thread should log if it deleted it | 15:48 |
| *** rpittau is now known as rpittau|afk | 15:51 | |
| clarkb | any zk errors? | 15:56 |
| corvus | clarkb: none that i can find | 15:56 |
| corvus | clarkb: this is a mystery and i am stumped. | 15:58 |
| *** DamianFajfer[m] is now known as fajfer | 16:01 | |
| fajfer | hey there | 16:01 |
| corvus | clarkb: i'm going to stop looking into it, and just see if it happens again and maybe we can identify a commonality or something. let me know if you have any other ideas. | 16:01 |
| corvus | clarkb: btw https://review.opendev.org/804304 is ready -- you reviewed at the parents | 16:02 |
| corvus | Damian Fajfer: hi! if you have a question or something you want to talk about, feel free to go right ahead :) | 16:04 |
| clarkb | corvus: that seems reasonable. Should we unenqueue and reeneuque it? | 16:04 |
| clarkb | I can do that if you think that is reasonable at this point | 16:07 |
| corvus | clarkb: is it interesting that it reported a result on the current patchset? | 16:07 |
| corvus | clarkb: oh sorry, wrong change | 16:08 |
| corvus | (ps2 of 804956 has not reported) | 16:08 |
| corvus | clarkb: yes, i think dequeue is good | 16:08 |
| clarkb | ok I'll do that | 16:08 |
| clarkb | via abandon and restore because that is easy | 16:09 |
| *** sshnaidm is now known as sshnaidm|afk | 16:09 | |
| fajfer | corvus: hey, thanks. I'd hate to be that guy so I realised I'd just hang out a bit first. I've been using Zuul for over a year now, just having some random trouble with a job not being queued properly (like if I didn't pass eg. branch criteria). If I copy the job from the config into the zuul.yaml it works flawlessly | 16:09 |
| fajfer | config project into the zuul.yaml untrusted project - just to make stuff clear | 16:09 |
| fungi | explicit vs implicit branch filters can be tricky. zuul will attempt to use configuration from untrusted projects by matching branch names between them (unless you override the branch matching). failing that, it will check the default branch (usually "master" or "main" depending on how the project is configured) | 16:13 |
| fungi | you can add explicit branch filters in projects, but that's usually only recommended to do when defining the job in a single-branch repository, as zuul does a better job at using branched configuration (reading configuration from the intended branch of the project) if the job is defined in a multi-branch repository | 16:14 |
| fungi | also having more than one branch in a trusted config project can lead to problems, i think it only uses configuration from the default branch of config project though i need to double-check the docs on that point | 16:15 |
| fajfer | the trusted config project uses only one branch | 16:15 |
| fajfer | and I think you're right | 16:16 |
| fungi | so anyway, you've got a (single-branch) config project defining a job with some explicit branch matcher declared, and you're adding that job in a project-pipeline for some project but it's not being triggered? | 16:17 |
| fajfer | the problem I got is I got a config project containing several jobs, and I got this development project which is untrusted. If the job definition remains in the config project, this sole job doesn't trigger in the pipeline. Copied over to the zuul.yaml on development project triggers properly | 16:19 |
| fungi | is there anything unique about the job which isn't running? like does it use secrets while the others don't? | 16:20 |
| fungi | or does it have a different inheritance path, and maybe there's something about its parent job which is the cause? | 16:21 |
| corvus | Damian Fajfer: if you're an admin, you can run the scheduler with debug logs enabled and zuul will log why it isn't running a job. as a user, you can set this value and get the same information in the report (you might need a noop job running to make it report): https://zuul-ci.org/docs/zuul/reference/project_def.html#attr-project.%3Cpipeline%3E.debug | 16:21 |
| corvus | Damian Fajfer: If it is related to branches, as fungi suspects, you might find pragmas interesting: https://zuul-ci.org/docs/zuul/reference/pragma_def.html | 16:22 |
| corvus | and also make sure you're familiar with the branch logic fungi describes -- https://zuul-ci.org/docs/zuul/reference/job_def.html#attr-job.branches | 16:23 |
| fungi | well, the only reason i got fixated on branches was fajfer's comment about "random trouble with a job not being queued properly [...] didn't pass eg. branch criteria" | 16:24 |
| fungi | but maybe i misunderstood and it was an example of prior challenges not related to the current one | 16:24 |
| corvus | it's a good guess | 16:24 |
| corvus | either way, logs should tell | 16:25 |
| fungi | certainly the branch matcher logic is some of zuul's more magical behavior | 16:25 |
| fungi | (or seems that way until you read the explanation in the docs anyway) | 16:25 |
| corvus | magical but not mysterious | 16:26 |
| fajfer | https://dpaste.com/4FYGKKQSK this is the structure I got | 16:34 |
| fajfer | I have a very similar job to this that works flawlessly in all variants, I just can't get this problematic-job to work | 16:35 |
| fajfer | I'm gonna try the logs for sure | 16:35 |
| fungi | if there are secrets involved, then job selection gets more complicated as zuul has a number of safeguards in place to make it hard for those to be triggered in situations where the secrets could be exposed by a user. you are adding it to a post-review pipeline, right? and where is the secret defined? | 16:42 |
| fajfer | similar to opendev - zuul.d/secrets.yaml in a config project | 16:43 |
| fajfer | and yeah I confirm this is in a post-review: true pipeline | 16:44 |
| fungi | and the playbooks which use that secret are defined in the config project as well in that case, and the parent jobs don't utilize the secret or you're using pass-to-parent explicitly for them? | 16:44 |
| fajfer | secrets have pass-to-parent: true | 16:45 |
| fungi | might be good to review the caveats listed at https://zuul-ci.org/docs/zuul/reference/secret_def.html if you haven't already, just to be sure the cause isn't one of those | 16:45 |
| fajfer | it works overall, I'm just really having some trouble with one job not being queued | 16:46 |
| fajfer | but feel free to ask away, I've read the docs, I don't seem to find anything that could help me :( | 16:46 |
| fungi | and you're not using an allowed-projects list with that job presumably? though that would probably have jumped out at you already if you were | 16:46 |
| corvus | it will be much more efficient to check the logs | 16:47 |
| fungi | yep, agreed | 16:47 |
| fungi | the reasons zuul might *not* run something are rather numerous | 16:47 |
| fajfer | yeah, but that's the first time I'm totally disoriented | 16:48 |
| fajfer | might need to increase the verbosity or something, I think I missed that one article | 16:48 |
| corvus | clarkb: i realized that the request is removed on the merger without subsequent log lines related to that on the scheduler, so our missing merge request from earlier was actually run and completed on ze04 | 16:50 |
| clarkb | ah | 16:51 |
| corvus | i will make log improvements in a bit; for now i'll track down the next link | 16:51 |
| corvus | clarkb: the merge failed: stderr: 'ssh: Could not resolve hostname review.opendev.org: Temporary failure in name resolution | 16:52 |
| fungi | that's a surprising error | 16:53 |
| clarkb | we've seen it before. It prompted us to bump the ttl on the record back to an hour | 16:53 |
| clarkb | corvus: I would expect that to cause zuul to report an error back at least? | 16:53 |
| corvus | yeah, i think it didn't submit a completed event; should be able to repro and fix | 16:54 |
| fungi | was that stderr from a task on a job node? we've also seen it happen more on providers where a v4 nat is in use (though generally we prefer resolution over ipv6 in those places) | 16:54 |
| clarkb | fungi: no on the zuul merger. The change didn't run any jobs because its initial merge never compelted | 16:54 |
| fungi | oh, this was from attempting to make a connection to the api? | 16:55 |
| fungi | neat | 16:55 |
| fungi | so one of our mergers had name resolution issues | 16:55 |
| clarkb | side note: dns resolution problems aren't related to the gerrit server move. I wonder if we're seeing flakier network in general | 16:55 |
| opendevreview | James E. Blair proposed zuul/zuul master: Merger related cleanup https://review.opendev.org/c/zuul/zuul/+/805254 | 16:59 |
| corvus | clarkb, fungi: actually, here's the log improvements first | 17:00 |
| fajfer | I just put the job into check instead to post to hasten my debugging and it queued | 17:03 |
| fungi | yeah since the job is defined in a config project, even though it uses a secret, it can be enqueued into a pre-review pipeline | 17:11 |
| fungi | because config projects don't get speculative execution of job configuration changes | 17:12 |
| fungi | so i guess there's something about the job which is causing it not to match the conditions of your post pipeline, but does match the conditions for your check pipeline | 17:14 |
| opendevreview | Merged zuul/zuul master: web: Job: don't display loading/refresh header https://review.opendev.org/c/zuul/zuul/+/804821 | 17:19 |
| opendevreview | James E. Blair proposed zuul/zuul master: Send merge completed events even in case of error https://review.opendev.org/c/zuul/zuul/+/805257 | 17:22 |
| clarkb | corvus: in ^ was the retry attempts code not working properly or did we fail to resolve the name 3 times in a row after waiting 30 seoncds between attempts? | 17:24 |
| corvus | clarkb, fungi, ianw, felixedel, swest, tobiash: see 805257 | 17:24 |
| corvus | clarkb: failed 3x | 17:27 |
| clarkb | wow | 17:27 |
| clarkb | also gerrit makes that diff look much larger than it really is | 17:28 |
| corvus | 23:52:31,825 23:53:11,905 and 23:53:21,969 | 17:28 |
| corvus | i can't immediately explain the math on the timing there :/ | 17:29 |
| corvus | probably has something to do with how long the timeouts took | 17:29 |
| clarkb | still much longer than you would hope a random udp problem would last for | 17:29 |
| corvus | and yeah, that diff is mostly an outdent | 17:29 |
| opendevreview | James E. Blair proposed zuul/zuul master: Remove a particularly verbose test debug log https://review.opendev.org/c/zuul/zuul/+/805258 | 17:29 |
| corvus | that ^ is a pet peeve | 17:30 |
| corvus | i keep commenting that out every time i start one of these repro tests :/ | 17:30 |
| opendevreview | Merged zuul/zuul master: Fix config cache cleanup with extra config paths https://review.opendev.org/c/zuul/zuul/+/805175 | 17:53 |
| * TylerPearce[m] < https://matrix.org/_matrix/media/r0/download/matrix.org/PoBRzjTBWHdyGQqIIdGwsxvE/message.txt > | 18:03 | |
| clarkb | TylerPearce[m]: did you make sure the PR was approved as that is listed as a requirement. The PR must also be in an open state | 18:05 |
| clarkb | Then you trigger the jobs by leaving a comment with 'merge' in it or label the PR merge is I am reading the pipeline definition properly | 18:06 |
| fungi | but switching the gate pipeline described in that example from manager: dependent to independent works even when not altering the require and trigger parameters? | 18:06 |
| TylerPearce[m] | Ah sorry, the "gate" pipeline always works. Just the "check" pipeline does not work when changing the manager to `dependent` | 18:08 |
| TylerPearce[m] | The check pipeline should trigger from a comment 'recheck', but this only seems to trigger if the manager is `independent`, regardless of approval/open status | 18:08 |
| fungi | ahh, so your "gate" pipeline in that example, which is set to manager: dependent, is already working | 18:08 |
| TylerPearce[m] | Yes the "gate" pipeline is working, I'm just trying to test some things on a dependent pipeline without actually merging anything | 18:09 |
| fungi | i wonder if the dependent pipeline manager implicitly checks for mergeable conditions | 18:10 |
| * TylerPearce[m] uploaded an image: (169KiB) < https://matrix.org/_matrix/media/r0/download/matrix.org/PTjJoCJOqMOCnAEaMeUYufSH/image.png > | 18:11 | |
| TylerPearce[m] | I tried commenting "recheck" on an approved/open PR which didn't seem to trigger anything, then "merge" right after which worked successfully. The merge conditions should have been the same for both triggers, I think | 18:11 |
| fungi | i don't see any mention of the dependent pipeline manager having special expectations as far as the github checks api driver has documented | 18:13 |
| TylerPearce[m] | Yeah I didn't see anything either, but I'm struggling to understand why my "check" pipeline isn't running here. I can work around this by just testing on the "gate" pipeline, I just like to know why things aren't working the way I expect them to XD | 18:14 |
| fungi | i've not seen the dependent pipeline manager used for pre-approval pipelines, so it's possible there's some nuance of the pipeline managers coming into play we've just not exercised there | 18:14 |
| fungi | usually dependencies in pre-approval testing are declared explicitly between changes or implicitly via git parenting, so i'm not sure how teh dependent manager would function in that scenario | 18:15 |
| fungi | i guess the desire is for your check pipeline to be sequenced based on trigger time? | 18:16 |
| fungi | i would expect that to create some significant delays in getting test results reported on open changes, but there's probably some workflow involved there i'm just not able to imagine without additional context | 18:17 |
| TylerPearce[m] | My goal here is to get a better understanding of how project gating (https://zuul-ci.org/docs/zuul/discussion/gating.html?highlight=project%20gating) works, so I was planning on experimenting with some stuff in a dependent pipeline | 18:18 |
| TylerPearce[m] | Ultimately my company wants to continue using CircleCI to run our test suites, so I need some way of communicating to CircleCI exactly what changes it should test | 18:18 |
| TylerPearce[m] | Using a dependent pre-approval pipeline was just for my own testing, the "check" pipeline I plan to use in production will be a standard independent pipeline | 18:19 |
| TylerPearce[m] | I can still run my experiments in the gate pipeline, so it's not an issue :) I was just curious if I was missing something obvious | 18:21 |
| fungi | it's an interesting experiment, i've just not seen the dependent pipeline manager applied to a check-like pipeline so not exactly sure what to expect. the lack of enqueuing is certainly an interesting outcome | 18:22 |
| fungi | we don't seem to document anything at https://zuul-ci.org/docs/zuul/reference/pipeline_def.html#value-pipeline.manager.dependent which would imply there's a system-wide expectation precluding what you're trying, but i wouldn't be entirely surprised to find that there could be assumptions in some of the connection drivers around triggering conditions | 18:26 |
| TylerPearce[m] | I'll poke around a little bit and report back if I find anything. Thanks for talking through this with me! | 18:28 |
| fungi | no worries, sorry i didn't have any better insights, but maybe someone else here has tried the same thing or knows about nuances of the github driver which could be complicating your experiment | 18:29 |
| opendevreview | Merged zuul/zuul master: Merger related cleanup https://review.opendev.org/c/zuul/zuul/+/805254 | 18:29 |
| opendevreview | Merged zuul/zuul master: Send merge completed events even in case of error https://review.opendev.org/c/zuul/zuul/+/805257 | 18:49 |
| pabelanger[m] | Tyler Pearce: we do alot of github testing too, https://github.com/ansible/project-config/blob/master/zuul.d/pipelines.yaml is you need to compare pipelines | 19:04 |
| pabelanger[m] | however, I've never done a dependant check pipeline | 19:04 |
| pabelanger[m] | if you add pull_request action trigger, you can open / close PRs to trigger the pipeline | 19:04 |
| pabelanger[m] | which may help | 19:04 |
| bridgefan | does anyone know if the default build status page would get replaced with log_url if log_url is available? https://opendev.org/zuul/zuul-jobs/src/branch/master/roles/upload-logs/tasks/main.yaml#L78? | 20:27 |
| clarkb | bridgefan: the log url becomes an attribute of the build page and does not replace it | 20:27 |
| bridgefan | just to make sure I understand... normally when connecting zuul to gerrit there is a url that is passed back on success/failure that points to the job's log folder... are you saying the log url somehow gets added to this file listing? | 20:29 |
| bridgefan | some time ago the success/fail url used to point to the job's console | 20:30 |
| bridgefan | I was hoping to redirect back to that | 20:30 |
| opendevreview | Merged zuul/zuul master: web: JobVariant : convert to DescriptionList https://review.opendev.org/c/zuul/zuul/+/804601 | 20:30 |
| clarkb | bridgefan: the build status page is being used in modern zuul (I'm not sure if the currently release has those changes but opendev's zuul does) to aggregate that info | 20:36 |
| clarkb | bridgefan: while the job is running you'll continue to get links to the console log. But once the job has fnished links are provided to the build status page | 20:36 |
| clarkb | then on the build status page you have access to all of the logs and other information | 20:37 |
| bridgefan | hm, I'm not sure why but at the moment that changed on our version of zuul | 20:39 |
| clarkb | bridgefan: for example for the change above that just merged the link for the zuul-tox-docs job go to https://zuul.opendev.org/t/zuul/build/7ed86b1fae0c48cf940fd7d9852840ce | 20:39 |
| clarkb | bridgefan: its possible that it made it into a release I haven't keep up with it since opendev has been running that way for a while now | 20:39 |
| bridgefan | I appreciate your help | 20:40 |
| bridgefan | the change is I think only from the last couple months | 20:40 |
| bridgefan | what you are showing is what we used to have | 20:41 |
| bridgefan | thanks | 20:41 |
| corvus | bridgefan, clarkb: https://zuul-ci.org/docs/zuul/reference/releasenotes.html#deprecation-notes | 20:42 |
| clarkb | hrm I guess I don't understand how it changed if the build page is what they had before and now dont have | 20:49 |
| corvus | me neither; that should be the only possibility | 20:52 |
| *** dviroel|ruck is now known as dviroel|ruck|out | 21:08 | |
| opendevreview | James E. Blair proposed zuul/zuul master: Several merger cleanups https://review.opendev.org/c/zuul/zuul/+/805309 | 21:42 |
| opendevreview | James E. Blair proposed zuul/zuul master: Several merger cleanups https://review.opendev.org/c/zuul/zuul/+/805309 | 21:48 |
| corvus | clarkb, fungi, felixedel, swest, tobiash: ^ that's me trying to find out if we have a systemic lock leak (i don't think so) | 21:50 |
| corvus | but those seemed like good improvements | 21:50 |
| opendevreview | James E. Blair proposed zuul/zuul master: Several merger cleanups https://review.opendev.org/c/zuul/zuul/+/805309 | 22:01 |
| opendevreview | James E. Blair proposed zuul/zuul master: Cleanup stale locks in merger/executor API https://review.opendev.org/c/zuul/zuul/+/805312 | 22:15 |
| corvus | clarkb, fungi, felixedel, swest, tobiash: ^ and that should clean up our locks | 22:16 |
| opendevreview | James E. Blair proposed zuul/zuul master: Cleanup stale locks in merger/executor API https://review.opendev.org/c/zuul/zuul/+/805312 | 22:16 |
| TylerPearce[m] | fungi: I think I figured out the wonkiness that was preventing my "check" pipeline from running with a "dependent" manager. I had previously required "check" and "gate" to pass as part of the branch protection rules in GitHub. I think the dependent pipeline considers these conditions when deciding if it should run, and my "gate" pipeline seemed to work because I had ran/passed the "check" pipeline just before running it. Once I removed the | 22:53 |
| TylerPearce[m] | branch protection rules, my dependent check pipeline was able to run successfully 👍️ | 22:53 |
| opendevreview | Merged zuul/zuul master: Remove a particularly verbose test debug log https://review.opendev.org/c/zuul/zuul/+/805258 | 23:03 |
| fungi | TylerPearce[m]: oh, that makes some sense, given what little i actually understand about github's permissions model and the github driver we have | 23:04 |
| fungi | thanks for following up! | 23:04 |
| clarkb | corvus: looking at the locks change now | 23:09 |
| clarkb | corvus: I've approved it but left a super minor nit on it | 23:12 |
| corvus | clarkb: did you see the parent? | 23:16 |
| clarkb | corvus: I did not. I have fried brain apparently. Looking | 23:16 |
| corvus | clarkb: i agree with your nit. in my defense, that ordering was a late refactor (it's like version 3 of that method) and so i wasn't thinking about it chronologically. but i also agree, it's not worth another ps. | 23:17 |
| clarkb | corvus: I've approved the parent now but also left a coupel of minor things there | 23:25 |
| clarkb | ianw: I like https://review.opendev.org/c/zuul/zuul/+/804956 now that i can see what it does via the site preview. I did leave a comment on one of the Job page changes. I'm not a fan of the two columns of stuff. Much prefer the single list of key: value pairs | 23:29 |
| corvus | Clark: replied. | 23:30 |
| clarkb | corvus: oh I see taht is what you meant in the commit message | 23:32 |
| ianw | clarkb: yeah, i responded, not sure it makes sense to call out the description if not using 2 columns. it's subjective, i don't really mind | 23:37 |
| clarkb | responded. I don't feel super strongly about it if others prefer that rendering | 23:39 |
Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!
