| corvus | clarkb: i'm game for a restart | 00:01 |
|---|---|---|
| opendevreview | James E. Blair proposed zuul/zuul-operator master: Add static node to functional test https://review.opendev.org/c/zuul/zuul-operator/+/799874 | 01:41 |
| opendevreview | xinliang proposed zuul/zuul-jobs master: Fix install podman error on Ubuntu aarch64 Bionic https://review.opendev.org/c/zuul/zuul-jobs/+/803413 | 02:17 |
| opendevreview | James E. Blair proposed zuul/zuul master: WIP: Refactor Merger/Executor API https://review.opendev.org/c/zuul/zuul/+/803529 | 03:09 |
| opendevreview | xinliang proposed zuul/zuul-jobs master: Fix install podman error on Ubuntu aarch64 Bionic https://review.opendev.org/c/zuul/zuul-jobs/+/803413 | 03:24 |
| opendevreview | James E. Blair proposed zuul/zuul master: Refactor Merger/Executor API https://review.opendev.org/c/zuul/zuul/+/803529 | 03:28 |
| opendevreview | xinliang proposed zuul/zuul-jobs master: Fix install podman error on Ubuntu aarch64 Bionic https://review.opendev.org/c/zuul/zuul-jobs/+/803413 | 03:57 |
| opendevreview | xinliang proposed zuul/zuul-jobs master: Fix install podman error on Ubuntu aarch64 Bionic https://review.opendev.org/c/zuul/zuul-jobs/+/803413 | 04:25 |
| *** marios is now known as marios|ruck | 06:31 | |
| *** jpena|off is now known as jpena | 07:08 | |
| opendevreview | xinliang proposed zuul/zuul-jobs master: Fix install podman error on Ubuntu aarch64 Bionic https://review.opendev.org/c/zuul/zuul-jobs/+/803413 | 07:26 |
| *** rpittau|afk is now known as rpittau | 07:47 | |
| opendevreview | Simon Westphahl proposed zuul/zuul master: Update Github SHA cache on PR update https://review.opendev.org/c/zuul/zuul/+/803544 | 08:15 |
| opendevreview | xinliang proposed zuul/zuul-jobs master: Fix install podman error on Ubuntu aarch64 Bionic https://review.opendev.org/c/zuul/zuul-jobs/+/803413 | 08:20 |
| swest | clarkb: can you have a look at 803544? maybe you still remember the details of the referenced change and if there was a reason for removing the update of the sha cache | 09:02 |
| opendevreview | xinliang proposed zuul/zuul-jobs master: Fix install podman error on Ubuntu aarch64 Bionic https://review.opendev.org/c/zuul/zuul-jobs/+/803413 | 09:35 |
| opendevreview | Dong Zhang proposed zuul/zuul master: Show emoji to highlight failed jobs in build result in Github https://review.opendev.org/c/zuul/zuul/+/803547 | 09:50 |
| opendevreview | xinliang proposed zuul/zuul-jobs master: Fix install podman error on Ubuntu aarch64 Bionic https://review.opendev.org/c/zuul/zuul-jobs/+/803413 | 10:40 |
| *** jcapitao is now known as jcapitao_lunch | 11:03 | |
| *** dviroel|out is now known as dviroel | 11:25 | |
| *** jpena is now known as jpena|lunch | 11:37 | |
| *** jcapitao_lunch is now known as jcapitao | 12:28 | |
| opendevreview | Tristan Cacqueray proposed zuul/zuul master: [web] Update the useContext state to handle multiple values https://review.opendev.org/c/zuul/zuul/+/803592 | 12:38 |
| opendevreview | Tristan Cacqueray proposed zuul/zuul master: [web] Add useSetTenant hook https://review.opendev.org/c/zuul/zuul/+/803593 | 12:38 |
| opendevreview | Tristan Cacqueray proposed zuul/zuul master: [web] Demonstrate a functional BuildPage using hooks https://review.opendev.org/c/zuul/zuul/+/803594 | 12:38 |
| opendevreview | Tristan Cacqueray proposed zuul/zuul master: [web] Update useRemoteData to return a refresh callback https://review.opendev.org/c/zuul/zuul/+/803595 | 12:38 |
| *** jpena|lunch is now known as jpena | 12:39 | |
| opendevreview | Matthieu Huin proposed zuul/zuul master: CORS: support regular expressions in allowed origins https://review.opendev.org/c/zuul/zuul/+/803209 | 13:21 |
| corvus | swest: any way we can add a test for that? | 13:21 |
| swest | corvus: I can check. not sure if I find time to do so today | 13:25 |
| corvus | swest: i think we can merge the fix without it, but it looks like the sort of thing that colud happen again | 13:27 |
| swest | agree | 13:27 |
| corvus | wow, mypy found 3 "errors" with https://review.opendev.org/803529 and it's wrong about all 3 of them :( | 13:32 |
| tristanC | corvus: i thought we stopped using them, shouldn't we remove the mypy check? | 13:37 |
| corvus | tristanC: i don't think we got around to that, but i'm starting to think we should; i can't get "# type: ignore" to work either | 13:38 |
| corvus | ah, it has to be on the same line | 13:41 |
| opendevreview | James E. Blair proposed zuul/zuul master: Refactor Merger/Executor API https://review.opendev.org/c/zuul/zuul/+/803529 | 13:42 |
| opendevreview | James E. Blair proposed zuul/zuul master: Stop running mypy in linters https://review.opendev.org/c/zuul/zuul/+/803602 | 13:44 |
| opendevreview | Dong Zhang proposed zuul/zuul master: Show emoji to highlight failed jobs in build result in Github https://review.opendev.org/c/zuul/zuul/+/803547 | 13:58 |
| clarkb | swest: compare patchsets 2 and 3 of the original change | 14:25 |
| clarkb | swest: my hunch is that the pr cache was added and that change conflicted with it. When resolving the conflict it was removed innappropriately? | 14:26 |
| opendevreview | Dong Zhang proposed zuul/zuul master: Show emoji to highlight failed jobs in build result in Github https://review.opendev.org/c/zuul/zuul/+/803547 | 14:26 |
| clarkb | yes I think that is exactly what happened. The commits were all proposed within a week of each other looks like | 14:29 |
| swest | clarkb: k, just wanted to make sure I'm not missing something there. I'll try to come up with a test for that | 14:30 |
| clarkb | swest: I just approved the change with a note about what I discovered. Should I unapprove it if you want to add testing? | 14:31 |
| corvus | testing in followup wfm | 14:31 |
| clarkb | k I'll leave it in an approved state then. Sorry about that | 14:32 |
| corvus | testing >> code review :) | 14:32 |
| corvus | a lot of us +2d that :) | 14:33 |
| swest | I'll be on vacation next week, but will push a follow up with a test hopefully the week after | 14:34 |
| opendevreview | James E. Blair proposed zuul/zuul-operator master: Add static node to functional test https://review.opendev.org/c/zuul/zuul-operator/+/799874 | 14:34 |
| opendevreview | James E. Blair proposed zuul/zuul-operator master: Mount connection sshkeys on executors and mergers https://review.opendev.org/c/zuul/zuul-operator/+/799998 | 14:35 |
| opendevreview | James E. Blair proposed zuul/zuul-operator master: Set component command with args instead of command https://review.opendev.org/c/zuul/zuul-operator/+/800263 | 14:35 |
| opendevreview | James E. Blair proposed zuul/zuul-operator master: Configure debug logs for merger https://review.opendev.org/c/zuul/zuul-operator/+/800264 | 14:35 |
| opendevreview | James E. Blair proposed zuul/zuul-operator master: Make nodepool external_config mount more generic https://review.opendev.org/c/zuul/zuul-operator/+/800786 | 14:35 |
| opendevreview | James E. Blair proposed zuul/zuul-operator master: Document externalConfig https://review.opendev.org/c/zuul/zuul-operator/+/800791 | 14:35 |
| opendevreview | James E. Blair proposed zuul/zuul-operator master: Fix config update detection https://review.opendev.org/c/zuul/zuul-operator/+/800991 | 14:35 |
| *** jpena is now known as jpena|off | 15:15 | |
| corvus | tristanC: when you have a moment, can you re-review https://review.opendev.org/799874 (ps 18->19) -- i implemented a change you suggested | 15:20 |
| opendevreview | Matthieu Huin proposed zuul/zuul master: CORS: support regular expressions in allowed origins https://review.opendev.org/c/zuul/zuul/+/803209 | 15:43 |
| opendevreview | Merged zuul/zuul master: Update Github SHA cache on PR update https://review.opendev.org/c/zuul/zuul/+/803544 | 16:03 |
| *** marios|ruck is now known as marios|out | 16:08 | |
| *** sshnaidm is now known as sshnaidm|afk | 16:29 | |
| *** rpittau is now known as rpittau|afk | 16:41 | |
| nirmoy | How do you debug periodic pipeline job failure, I have a hourly periodic job that seems to never get triggered. only log I found in zuul scheduler is " WARNING zuul.Scheduler: No old pipeline matching periodic-hourly found when reconfiguring" | 16:43 |
| avass[m] | nirmoy: are there any config errors reported in the dashboard for that tenant? | 16:59 |
| opendevreview | Matthieu Huin proposed zuul/zuul master: [api][cors] Add CORS configuration https://review.opendev.org/c/zuul/zuul/+/767691 | 17:02 |
| nirmoy | avass: No, just says: Events: 0 trigger events, 0 management events, 0 results. | 17:06 |
| corvus | nirmoy: you may want to run the scheduler with debug logging ("-d") which may provide more info on what it's not doing | 17:07 |
| avass[m] | nirmoy: no bell icon in the top right? | 17:08 |
| nirmoy | avass: Yes, no bell icon. | 17:09 |
| nirmoy | corvus: thanks, let me check if I can enable debug logging. | 17:10 |
| opendevreview | Merged zuul/zuul-operator master: Add static node to functional test https://review.opendev.org/c/zuul/zuul-operator/+/799874 | 17:42 |
| opendevreview | Merged zuul/zuul-operator master: Mount connection sshkeys on executors and mergers https://review.opendev.org/c/zuul/zuul-operator/+/799998 | 17:42 |
| opendevreview | Merged zuul/zuul-operator master: Set component command with args instead of command https://review.opendev.org/c/zuul/zuul-operator/+/800263 | 17:42 |
| opendevreview | Merged zuul/zuul-operator master: Configure debug logs for merger https://review.opendev.org/c/zuul/zuul-operator/+/800264 | 17:42 |
| clarkb | felixedel: corvus: I have left comments on https://review.opendev.org/c/zuul/zuul/+/802299 I am going to grab lunch but happy to discuss those if they need clarification | 18:55 |
| -opendevstatus- NOTICE: The Gerrit service on review.opendev.org is going down for a quick restart to adjust its database connection configuration, and should return to service momentarily | 20:04 | |
| corvus | clarkb: i have responded with some more info/context on things i immediately know; most of the comments you left that i didn't respond to i agree with but would like to double check when i can next update my working tree. but maybe you could give the responses a quick look when you have a sec and see if more is needed there? | 20:43 |
| clarkb | corvus: ya I'll take a look shortly | 20:44 |
| opendevreview | James E. Blair proposed zuul/zuul master: Add commands to export/import keys to/from ZK https://review.opendev.org/c/zuul/zuul/+/800854 | 21:05 |
| opendevreview | Tristan Cacqueray proposed zuul/zuul master: Fix support for multiple github connection https://review.opendev.org/c/zuul/zuul/+/803652 | 21:05 |
| tristanC | pabelanger[m]: here is a change to fix the issue we were having with ansible-collections ci ^ | 21:06 |
| clarkb | corvus: repsonded. Your comments make sense to me | 21:36 |
| opendevreview | Tristan Cacqueray proposed zuul/zuul master: Fix support for multiple github connection https://review.opendev.org/c/zuul/zuul/+/803652 | 22:20 |
| opendevreview | James E. Blair proposed zuul/zuul master: Execute merge jobs via ZooKeeper https://review.opendev.org/c/zuul/zuul/+/802299 | 22:26 |
| corvus | clarkb: ^ a few more responses and updates | 22:28 |
| opendevreview | James E. Blair proposed zuul/zuul master: Refactor Merger/Executor API https://review.opendev.org/c/zuul/zuul/+/803529 | 22:28 |
| opendevreview | James E. Blair proposed zuul/zuul master: Stop running mypy in linters https://review.opendev.org/c/zuul/zuul/+/803602 | 22:28 |
| corvus | (and rebases ^) | 22:28 |
| clarkb | corvus: re the test job history stuff what is RecordingMergeClient then? | 22:37 |
| clarkb | I thought its submit job and history recording (which uses type not uuid) is what we were getting there because the schedulers are merge clients? | 22:37 |
| clarkb | I suspect some of my confusion is related to the merger api doubling as both the client and server implementation so it isn't always claer at first glance which side you're getting | 22:39 |
| clarkb | but I agree now because we are hitting merger.merger_api that we get the holdable class and not the recording classes history | 22:40 |
| clarkb | corvus: ok responded. In a few places I was just confused or misread things. SOrry about that. But now I've got a new confusion and one suggestion. Can you take a look? | 22:54 |
| clarkb | also one thing I'm noticing is that all of your comments are in the resolved state is that a gertty behavior? I wonder if these changes with tons of comments would be easier to follow if we waited until everyone was happy with the comment thread to tick the rseolved box | 22:55 |
| clarkb | (its a thing that upstream gerrit seems to rely on a bit and I wonder if it would help us) | 22:55 |
| corvus | clarkb: what's the etiquite regarding resolved? who sets it? the originator or can other people? | 23:18 |
| corvus | clarkb: and gertty doesn't do anything with resolved, so that's just what gerrit defaults to | 23:18 |
| corvus | i'm sure i could change it, but i don't know how to use it so i haven't. | 23:19 |
| clarkb | corvus: I think the general idea is someone leaves a comment and then you can go back and forth until there is agreement then whoever is the last commenter can check the resolved option | 23:22 |
| clarkb | in some cases that might be the person who left the original comment like i could resolve the comment about needs_results after clarification and agreement | 23:22 |
| clarkb | and in others it would be when you push a new patchset that fixes a problem as prescribed you could set it to resolved | 23:22 |
| corvus | okay. i guess i've just seen too many people leave things unresolved in gerrit's gerrit to be convinced that it's a worthwhile feature :/ | 23:24 |
| clarkb | yes, I think it would require asking people to actually use it (since it is a new featuer afterall) | 23:24 |
| clarkb | it also isn't critical | 23:25 |
| corvus | clarkb: replied | 23:27 |
| fungi | having ended up interfacing with google docs more often than i would like (which is, to be clear, at all), the "resolved" status for comments in gerrit seems entirely derivative of the g'docs editorial comment workflow | 23:27 |
| fungi | in reflection, maybe this shouldn't surprise me | 23:28 |
| opendevreview | James E. Blair proposed zuul/zuul master: Execute merge jobs via ZooKeeper https://review.opendev.org/c/zuul/zuul/+/802299 | 23:28 |
| clarkb | corvus: I have +2'd the change. Thank you for going through that with me | 23:29 |
| opendevreview | James E. Blair proposed zuul/zuul master: Refactor Merger/Executor API https://review.opendev.org/c/zuul/zuul/+/803529 | 23:29 |
| opendevreview | James E. Blair proposed zuul/zuul master: Stop running mypy in linters https://review.opendev.org/c/zuul/zuul/+/803602 | 23:29 |
| clarkb | oh wait you just updated it /me looks again :) | 23:29 |
| corvus | clarkb: thank you :) | 23:29 |
| corvus | i just did the test update you suggested | 23:29 |
| corvus | i'm looking at the unlock handling now, but that'll be a followup | 23:30 |
| clarkb | corvus: yup still lgtm my +2 will stay :) | 23:30 |
| opendevreview | Merged zuul/gating.dev master: Add initial job framework https://review.opendev.org/c/zuul/gating.dev/+/801901 | 23:33 |
| opendevreview | Merged zuul/gating.dev master: Add initial content https://review.opendev.org/c/zuul/gating.dev/+/801919 | 23:34 |
| corvus | clarkb: i'm with you on the unlock symmetry thing, but i also don't see a great way to do that with the executor right now. and i really like the executor/merger symmetry. i'd like not to spend cycles on it unless you think it's really important. also, if you want to take a look at it, maybe you'll have an idea? the executor is definitely trickier than the merger; that's the place to start. | 23:34 |
| corvus | clarkb: i'll also muse on it a bit more and let you know if something comes to mind. | 23:34 |
| clarkb | corvus: ya let me pull up the executor and see | 23:35 |
| corvus | it's the completions from the AnsibleJob that make that tricky | 23:36 |
| clarkb | corvus: this is execute() in the executor server that I should be looking at? | 23:36 |
| corvus | clarkb: runBuildWorker in executor == runMerger in merger | 23:37 |
| corvus | server.py of each | 23:37 |
| clarkb | corvus: what if we kept the merger and the executor the same and moved completeMergeJob() only to runMerger() instead of having it in runMerger and _runMergeJob()? | 23:38 |
| clarkb | then the two classes are in sync with each other if not fully symmetric within themselves | 23:38 |
| clarkb | we would need to return result from _runMergeJob to runMerger to do that | 23:39 |
| corvus | yeah, that could do it | 23:39 |
| corvus | clarkb: you want to do that or want me to take a stab? | 23:39 |
| clarkb | up to you. Though I probably won't be able to do that until tomorrow at this rate. We've got furniture to move and dinner to start and I'm upgrading gitea real quick | 23:40 |
| clarkb | I'm happy to do it, but won't be sad if you do it :) | 23:40 |
| corvus | clarkb: er... except that we also might not get the lock, in which case we shouldn't call completeMergeJob | 23:45 |
| clarkb | hrm does that mean we need to only call it in _runMergeJob() and have teh try except cover more of that method? | 23:46 |
| clarkb | oh I see we can't make it finally | 23:47 |
| corvus | that could be an option | 23:47 |
| clarkb | we should only call it in the case of an exception which can only happen after the lock is held | 23:47 |
| clarkb | ya I guess calling it in both places may be hte easiest thing | 23:48 |
| clarkb | (the current code) | 23:48 |
| opendevreview | James E. Blair proposed zuul/zuul master: Streamline unlocking in merger and builder run loops https://review.opendev.org/c/zuul/zuul/+/803657 | 23:51 |
| corvus | clarkb: maybe that ^? | 23:51 |
| corvus | er, there's an oops in there, ignore the 'return false' change | 23:52 |
| opendevreview | James E. Blair proposed zuul/zuul master: Streamline unlocking in merger and builder run loops https://review.opendev.org/c/zuul/zuul/+/803657 | 23:52 |
| opendevreview | James E. Blair proposed zuul/zuul master: Streamline unlocking in merger and builder run loops https://review.opendev.org/c/zuul/zuul/+/803657 | 23:53 |
| corvus | and somehow i missed a rebase | 23:53 |
| Clark[m] | corvus: I like that | 23:57 |
| corvus | yeah, i think that's closest in spirit to what you were originally suggesting | 23:58 |
Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!