| *** dkranz has joined #zuul | 01:00 | |
| *** SotK has quit IRC | 01:06 | |
| *** SotK has joined #zuul | 01:06 | |
| *** yolanda has quit IRC | 01:55 | |
| *** SotK has quit IRC | 02:22 | |
| *** SotK has joined #zuul | 02:25 | |
| SpamapS | weird... I can't get gear's test suite to run | 03:06 |
|---|---|---|
| *** bhavik1 has joined #zuul | 04:48 | |
| *** Cibo has joined #zuul | 09:05 | |
| *** Cibo_ has quit IRC | 09:07 | |
| *** Cibo_ has joined #zuul | 09:10 | |
| *** Cibo has quit IRC | 09:11 | |
| *** Cibo_ has quit IRC | 10:07 | |
| *** bhavik1 has quit IRC | 10:17 | |
| *** jkilpatr has quit IRC | 10:42 | |
| *** jkilpatr has joined #zuul | 11:10 | |
| Shrews | morning | 12:20 |
| tobiash | evening ;) | 12:33 |
| tobiash | (almost) | 12:33 |
| *** Cibo_ has joined #zuul | 12:34 | |
| tobiash | are there any objections adding a max-ready-age setting to the label config and cleaning up nodes which are longer than that in ready state? | 12:35 |
| tobiash | I have two use cases for this, first ensure that the slaves are not older than e.g. one hour (for e.g. forcing a recent cache) | 12:37 |
| tobiash | second we try to scale down to a minimum during non-office times | 12:37 |
| tobiash | so we have a config for office hours with a high min-ready and a config for non-office hours with min-ready of 1 | 12:37 |
| tobiash | this would also allow for nodepool initiated scale down of unneeded nodes | 12:38 |
| *** dkranz_ has joined #zuul | 12:40 | |
| *** dkranz has quit IRC | 12:40 | |
| openstackgerrit | David Shrewsbury proposed openstack-infra/nodepool feature/zuulv3: Fix CleanupWorker exception messages https://review.openstack.org/461019 | 12:44 |
| tobiash | something like http://paste.openstack.org/show/608324/ | 12:47 |
| tobiash | the work could then be done by one of the existing cleanup worker threads or a separate one | 12:55 |
| Shrews | tobiash: so you're really talking about 2 things: scaling down when min-ready changes, AND having a max-ready-age | 13:10 |
| tobiash | Shrews: yes, both could be done by max-ready-age (where scale down would be delayed by max-ready-age) | 13:20 |
| Shrews | tobiash: true | 13:21 |
| Shrews | i can't see any immediate problem with implementing that, but pabelanger and jeblair should probably weigh in from an ops perspective | 13:23 |
| mordred | Shrews, tobiash: sounds like a good addition to me - but I also agree, i'd lke pabelanger and jeblair to chime in | 13:29 |
| jeblair | yeah, max-ready-age sounds good. | 13:33 |
| tobiash | :) | 13:34 |
| tobiash | then I'll try that | 13:35 |
| tobiash | thx | 13:35 |
| tobiash | patch proposal will follow probably next week | 13:35 |
| *** Cibo_ has quit IRC | 13:49 | |
| *** dmsimard is now known as dmsimard|cavern | 14:44 | |
| pabelanger | max-ready-age seems fine | 14:49 |
| clarkb | refular cleanup should already do that I think? | 14:58 |
| clarkb | set the max age and cleanup frequency and that should do it? | 14:58 |
| jeblair | clarkb: we only have a max age for images, not nodes | 14:59 |
| pabelanger | indeed! When you have to pay for cloud resources out of pocket, it is nice to have a way to limit nodes for off hours :) | 15:12 |
| clarkb | jeblair: ah | 15:15 |
| *** openstackgerrit has quit IRC | 15:17 | |
| *** Cibo_ has joined #zuul | 15:22 | |
| *** Cibo_ has quit IRC | 15:42 | |
| *** yolanda has joined #zuul | 15:43 | |
| eventingmonkey | Hey SpamapS | 15:56 |
| SpamapS | eventingmonkey: so, you were thinking of tackling https://storyboard.openstack.org/#!/story/2000879 yeah? | 15:56 |
| eventingmonkey | Yes, I am. | 15:56 |
| SpamapS | eventingmonkey: so what we need is a thread running in the zuul executor that walks the job dirs and if there is a job that has exceeded the defined limit per-job of bytes on disk, kill the job. | 15:56 |
| SpamapS | and kill it softly enough that it cleans up its job dir ;) | 15:57 |
| SpamapS | eventingmonkey: so you'll need to understand a) how to kill jobs (there's a way) and b) how to find out all the running jobs and their job dirs (I believe that may only be held in memory) | 15:58 |
| eventingmonkey | Killing it softly with words. Got it. | 15:58 |
| SpamapS | or with your song | 15:58 |
| pabelanger | couldn't we use tmpreaper for that? I was actually considering adding http://git.openstack.org/cgit/openstack-infra/puppet-tmpreaper to our zuul-executors to help | 15:58 |
| SpamapS | pabelanger: that's the suspenders that helps with wayward executors | 15:58 |
| pabelanger | I guess this is to protect against a single jobs from filling HDD? | 15:58 |
| SpamapS | pabelanger: but this is for a perfectly healthy executor that knows all the jobs that are running, and yet, one of the jobs has exceeded its allocated storage. | 15:59 |
| SpamapS | Note that we had a pretty good discussion about this in Atlanta | 15:59 |
| SpamapS | and we probably should have written it down as a spec | 15:59 |
| eventingmonkey | lol | 15:59 |
| pabelanger | Ya, remembering now | 15:59 |
| SpamapS | but basically, like SELinux, the best system ways to do this are hard for operators to deploy | 16:00 |
| SpamapS | Like ideally, we'd just have an LVM volume group and mount each job dir as an LV with a defined size. | 16:00 |
| SpamapS | a thin LV too, so that we can overcommit | 16:00 |
| pabelanger | does bubblewrap profile file limits for the chroot too? | 16:00 |
| pabelanger | s/profile/provide | 16:00 |
| SpamapS | no, bubblewrap doesn't really know how to do that. | 16:00 |
| pabelanger | ack | 16:01 |
| SpamapS | and there's no ulimit for bytes on disk | 16:01 |
| SpamapS | and quotas are per-user (except in XFS, which has per-project, which dirs can be marked with xattrs, but.. XFS) | 16:01 |
| pabelanger | I thought clarkb or fungi also mentioned lvm might do something too? | 16:02 |
| SpamapS | pabelanger: before I take eventingmonkey monkey too far down this road.. isn't there a pile of ansible to write still? | 16:02 |
| eventingmonkey | SpamapS: these things are all in memory...care to help by putting them in words? | 16:02 |
| SpamapS | pabelanger: LVM is perfect for this. But setting it up requires quite a bit of work. | 16:02 |
| clarkb | zfs too | 16:02 |
| clarkb | but thats an even bigger mess | 16:02 |
| SpamapS | yyyyyyyyyyeeeeeeeeeeeeeeeaaaaaah | 16:02 |
| pabelanger | coolio! | 16:02 |
| SpamapS | So one alternative method we could explore.. | 16:03 |
| clarkb | but zfs totally has the dir quota set command | 16:03 |
| SpamapS | is having Zuul throw a big loopback on disk if you don't give it a block device. | 16:03 |
| SpamapS | and just carve out job dirs from a vg created on that loop | 16:03 |
| SpamapS | clarkb: yeah, I forgot zfs, because.... I forgot zfs. ;) | 16:03 |
| clarkb | SpamapS: you need more freebsd in your life :P | 16:03 |
| * fungi bets butterfs does too. kitchen sink and all | 16:03 | |
| * eventingmonkey is still surprised btrfs didn't catch on more | 16:04 | |
| pabelanger | SpamapS: ya, that's the discussion I am remembering about loopback on disk | 16:04 |
| SpamapS | one problem with doing it that way | 16:05 |
| SpamapS | is it requires root | 16:05 |
| eventingmonkey | SpamapS and pabelanger: So...should I dive into this one, or start poking at ansible? | 16:05 |
| *** openstackgerrit has joined #zuul | 16:05 | |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Use the security context of the playbook when checking out roles https://review.openstack.org/461092 | 16:05 |
| SpamapS | up until now, we haven't required root to do executor's job | 16:05 |
| SpamapS | eventingmonkey: as we're talking this out, I'm wondering if this is a bit more complicated than I'd originally thought | 16:06 |
| eventingmonkey | SpamapS: Yeah, I'm getting that impression... | 16:07 |
| pabelanger | jeblair: we need to land https://review.openstack.org/#/c/459728/ before we can start up zuulv3-dev again right? | 16:07 |
| jlk | jeblair: mordred: SpamapS: ya'll around to chat naming issues for github events? | 16:11 |
| jeblair | pabelanger: i don't think so, but we should land it soon. i will probably rip out that code in the not too distant future. | 16:12 |
| jeblair | jlk: yes! | 16:12 |
| * jlk pulls up the review tree | 16:13 | |
| SpamapS | jlk: I'm here. | 16:13 |
| jeblair | mordred, clarkb, SpamapS, jlk: before i forget, https://review.openstack.org/461092 can use some good security brain thought when you have a minute. i think that is correct now. (of course, it could also use tests of each of the cases) | 16:14 |
| SpamapS | and I think I'm in the "pass through unless it makes users go o_O" camp at this point. | 16:14 |
| jlk | (opened that tab for later) | 16:14 |
| * SpamapS will try and find a good security brain somewhere ;) | 16:14 | |
| jlk | okay https://review.openstack.org/#/c/439834/ is where I think the first feedback came in | 16:15 |
| jlk | where we're considering what to do with the events. | 16:15 |
| jlk | I'm pulling up my code, one sec | 16:15 |
| pabelanger | jeblair: okay, I'm not sure where we stand about bring zuulv3-dev.o.o back online. Is there anything stopping us from doing so? | 16:16 |
| jeblair | yep, briefly, that's the one where it looks really simple: "hey just pass them through, dude". then later changes make the question stickier. :) | 16:16 |
| jeblair | pabelanger: i'm not aware of any | 16:16 |
| jlk | https://github.com/j2sol/zuul/blob/github-v3/zuul/driver/github/githubconnection.py (yes github, but I couldn't find a gitweb link easily) | 16:16 |
| jeblair | pabelanger: i think we landed the immediately necessary fixes | 16:16 |
| jlk | that's the current state of the file. | 16:16 |
| pabelanger | jeblair: great, I'll pip install latest version of zuul and bring it back online | 16:17 |
| jlk | https://developer.github.com/webhooks/#events is the set of events Github could send us | 16:17 |
| jeblair | and here's an etherpad: https://etherpad.openstack.org/p/7HFyMRf519 | 16:17 |
| jlk | oh cool | 16:18 |
| jlk | right so lots are buried behind a "pull_request" event, namely (un)label, (re)opened, closed, syncronized (content changes). | 16:19 |
| SpamapS | I'm missing context | 16:19 |
| SpamapS | what makes mechanically just passing them through with .'s between object.action complicated later? | 16:20 |
| jeblair | i think the mechanical translation is pretty straightforward for pull_request | 16:20 |
| jlk | but the ohter events we care about is status (this is like a CI vote), pull_request_review (this is like a human vote), issue_comment (this is somebody commenting on the pull request), and maybe create (for tags being created, but that's out of scope for today) | 16:20 |
| jlk | so pull_reqeust.label pull_request.closed etc.. | 16:20 |
| jlk | and really, what we're talking about is what we expose in the layout file, right? | 16:21 |
| jlk | the thing that consumers will interact with? | 16:21 |
| jeblair | jlk: yes | 16:21 |
| jeblair | yep. and honestly, i thought i had a pretty good handle on what this would look like until i got to "issue_comment" | 16:22 |
| jlk | yeah... | 16:22 |
| jlk | so pull requests are "special" issues | 16:22 |
| jeblair | that's the one where i throw up my hands and curse at github | 16:22 |
| jlk | every PR Is an issue, but not every issue is a PR | 16:22 |
| clarkb | and in fact to close a PR you ahve to go through the issue api not PR api | 16:22 |
| clarkb | its really :( | 16:22 |
| jlk | now, we could mask that | 16:22 |
| jlk | and fake a pull_request.comment | 16:22 |
| jlk | internally we'd trap the issue_comment, see if it's a pull request, and present it up the stack as a pull_request.comment | 16:23 |
| jlk | jeblair: how do you see this looking in the layout, to capture the sub pull-request elements? Could you toss something quickly in the etherpad? | 16:26 |
| jeblair | yep. i think the pr/issue thing is confusing enough for new users that it's worth seriously considering that. i guess the question is, which is easier to read: the one that says 'pr-comment' and we have an asterisk in the docs that says "this is really the issue_comment github hook" or the one that says "issue_comment" with an asterisk in the zuul yaml that says "pull requests are issues". | 16:26 |
| jeblair | jlk: yeah, i'll type things | 16:26 |
| jeblair | ha, habitual typing of 'gerrit' :) | 16:27 |
| SpamapS | +1 for the * | 16:27 |
| jlk | even as a user of Github for years, it was surprising to me that PRs are issues. I think pr-comment would be easier for github users to grasp. We could asterisk it somewhere that we're injesting issue-comment and translating it, but it's really just for the pedants who know the API who would otherwise get upset. | 16:27 |
| clarkb | you'll also notice that issues and PRs share the same id sequence | 16:29 |
| jeblair | okay, style (A) and (B) are, i think, fairly reasonable examples of what that would look like. they both need a separate event stanza so that the 'comment' comparison is only applied to the comment event. | 16:31 |
| openstackgerrit | Paul Belanger proposed openstack-infra/zuul feature/zuulv3: WIP: Create stdlib.prepare-workspace role https://review.openstack.org/459066 | 16:31 |
| jeblair | so it's really only the one line that would change. | 16:31 |
| jlk | Yeah, I think I'm still in favor of B | 16:31 |
| jlk | if I see A, I'm going to immediately ask "what issue?" | 16:32 |
| clarkb | one thing to keep in mind is that users are likely to configure pipelinse very rarely. So either option is likely workable | 16:32 |
| jeblair | so it sounds like we're all in favor of saying issue_comment is bonghits enough that it crosses the line into something we need to mask. i can dig it. | 16:32 |
| openstackgerrit | Paul Belanger proposed openstack-infra/zuul feature/zuulv3: WIP: Create stdlib.prepare-workspace role https://review.openstack.org/459066 | 16:32 |
| jeblair | clarkb: yes, and probably quite a bit from copy pasta | 16:32 |
| pabelanger | okay, zuulv3-dev.o.o is back online | 16:32 |
| jeblair | especially if we do a good job writing the manual :) | 16:33 |
| jlk | pabelanger: \-/ | 16:33 |
| jeblair | jlk: is that geordi raising his arms? | 16:33 |
| jlk | I think it was supposed to be a pint glass of beer or something. | 16:33 |
| jlk | but I'm not so good with the ASCII | 16:33 |
| jeblair | oh, i can see a whiskey shot | 16:34 |
| jeblair | okay, so the other icky thing is the push/tag issue | 16:34 |
| jlk | |~| <--- rocks glass. | 16:34 |
| jlk | right. | 16:34 |
| jlk | So, we _could_ have zuul trigger whenever code is pushed to a specific branch | 16:35 |
| jlk | like, push to master, trigger some kind of deployment thingy | 16:35 |
| jlk | or regression tests | 16:35 |
| jlk | that's outside of a "pull request" realm, and more into just "repository actions" realm | 16:35 |
| jeblair | jlk: yep. and this is a surprisingly direct mapping to gerrit's ref-updated event. | 16:36 |
| jeblair | which we use for several pipelines in openstack | 16:36 |
| clarkb | jeblair: re the security related change you posted. Would it be simpler to just not allow trusted repos to dep on untrusted repos? | 16:36 |
| jlk | cool. We could even keep the same name yeah? It's not super user friendly, but... probably okay if it's used elsewhere in zuul | 16:36 |
| * mordred reads scrollback | 16:38 | |
| pabelanger | just noticed a new exception in zuul_stream.py | 16:40 |
| jeblair | jlk: well, i'm of two minds here... first, i kind of want to stick to the mechanical passthrough when possible. but two, the push/tag split that the github patches do is potentially useful. | 16:40 |
| pabelanger | http://paste.openstack.org/show/608361/ | 16:40 |
| jeblair | clarkb: lemme get back to you on that | 16:40 |
| pabelanger | looks like possible decode issue | 16:40 |
| jlk | jeblair: oh I think I see. Because ref-updated on gerrit catches tags too? | 16:40 |
| jeblair | jlk: yes, just like github push, actually. | 16:41 |
| jeblair | (the github patches take the github "push" event and split it out into either a "push" or "tag" event depending on what the target ref is) | 16:41 |
| jlk | yeah, makes regexes harder | 16:41 |
| jlk | I bet github originally just sent push, and then later broke out the tag event, but kept tags going through push for backwards compat | 16:42 |
| jeblair | jlk: no that's the thing -- github doesn't have a tag event | 16:42 |
| jeblair | that's a construction of, i guess, Jan's | 16:42 |
| jlk | well | 16:43 |
| mordred | yah - github is the same as gerrit here - it sends an event called "push" (which gerrit sends as "ref-updated") | 16:43 |
| jlk | it has a "create" event | 16:43 |
| jlk | when a branch or tag is created | 16:43 |
| jlk | not the same as a tag getting updated tho :/ | 16:43 |
| jeblair | if you look at 443947 githubconnection.py lines 88-91, that's where it conditionally creates a push or tag event | 16:43 |
| mordred | a tag should never be updated :) | 16:43 |
| jeblair | are we sure those things aren't all just push events as well? | 16:44 |
| jlk | https://github.com/j2sol/zuul/blob/github-v3/zuul/driver/github/githubconnection.py#L91 | 16:44 |
| jlk | https://developer.github.com/v3/activity/events/types/#createevent | 16:44 |
| jeblair | like, i wonder if, when you create a branch, you get a "create" and a "push" event? | 16:44 |
| jeblair | or do you just get "create", and "push" only happens on update? | 16:45 |
| jlk | it looks remarkably like push event | 16:45 |
| jlk | but with fewer fields | 16:45 |
| jlk | jeblair: good question, I can test that real quick | 16:45 |
| jeblair | cool, i'll type more in etherpad while you do that | 16:45 |
| jlk | pusing a new branch did get a "create" event | 16:47 |
| jlk | and a push event | 16:47 |
| jlk | you get both | 16:48 |
| jeblair | we're rich! | 16:48 |
| jlk | same for tags | 16:48 |
| jeblair | setting aside create for the moment, i added two new stanzas to the bottom of the etherpad | 16:49 |
| jeblair | the configuration for a 'post-merge' pipeline (even though you might choose to use pull_request.closed on github for this, this pretty much looks the same as what you'd do for 'change landed on master') | 16:49 |
| jlk | yeah that could work. I don't see a strong need to split ref update / tag into separate events | 16:49 |
| jeblair | and then the configuration for a 'tag' pipeline | 16:49 |
| jlk | pr closed might be something different | 16:50 |
| jeblair | basically you just always have to use a regex in there. and with github, the regex is pretty straightforward. | 16:50 |
| jlk | keying off of "when new code hits this branch" is, I think, a better mind set. | 16:50 |
| jeblair | though, you do have to at least understand a little bit about git. | 16:50 |
| mordred | we have found that people understand git _way_ less than the zeitgeist would have us believe | 16:51 |
| jeblair | the regex is pretty ugly for gerrit, because it does something weird to the ref if it's a branch (it drops 'refs/heads/') | 16:51 |
| jeblair | mordred: yeah | 16:51 |
| jlk | yeah this is pretty straight forward, and fairly easy to document in our documentation. | 16:51 |
| jlk | a branch is in refs/heads and a tag is in refs/tags | 16:51 |
| jlk | and if somebody wants to do crazy ass things with the git driver and be able to trigger on OTHER things in refs/, continuing to use a regex seems best | 16:52 |
| jeblair | so i think there's a compelling case for "make some simple synthetic events for 'branch head was updated' and 'tag was updated'". but i think that needs to happen for both drivers, and i think we need to make sure we don't accidentally make it hard to trigger on refs/notes/foo. | 16:52 |
| * jlk imagines somebody keying off of a git note in refs/notes/ | 16:52 | |
| mordred | yes. this is sort of what I was thinking in my brainhole | 16:53 |
| clarkb | you could even use it against refs/changes | 16:53 |
| jeblair | so i'm thinking we should limit ourselves to passthrough the event as-is for now, effectively requiring regexes, document them so people can copy-pasta them, then later, maybe add synthetic events to both drivers | 16:53 |
| mordred | maybe we make three synthetic events: push, tag and ref - and we put any ref-updated/push events that don't match head or tag into ref? | 16:53 |
| jlk | I'm getting twitchy on "push" | 16:54 |
| jlk | because you push a tag | 16:54 |
| jlk | these actions are hard to classify | 16:54 |
| mordred | jlk: head, tag and ref? | 16:54 |
| jlk | branch, tag, ref ? | 16:54 |
| mordred | yah - I hearyou | 16:54 |
| jeblair | jlk: yeah, i think it makes sense to have the passthrough event called 'push'. it really is all of them. | 16:54 |
| jlk | nod | 16:54 |
| mordred | branch, tag and push? | 16:54 |
| jlk | that could be it. | 16:55 |
| jlk | branch: master, tag: release-*, push: regexmebaby | 16:55 |
| mordred | the reason I like synthetic events is that the associated operations on them are git operations, not necessarily gerrit or github operations | 16:55 |
| jeblair | mordred: my variant on your proposal is that we keep the passthrough events (ref-updated, and push, respectively). then we add "branch-updated" and "tag" or whatever as additional synthetic events later. i think we would end up firing both events in zuul in that case. | 16:55 |
| mordred | jeblair: I think that could work nicely if we're ok firing both events | 16:56 |
| jeblair | (er, both events in the case of someone pushing a tag. we'd get 'ref-updated'/'push' as well as 'tag') | 16:56 |
| jeblair | (obvs only one event in the case of someone pushing /refs/notes/foo) | 16:56 |
| mordred | it's mostly that things like responding to "branch-created" vs. "branch-updated" all involve some ugly git foo - whereas the other tihngs "pr-created" make sense to a normal user | 16:57 |
| jeblair | mordred: yeah, i think the cost of that is small, and we can avoid backwards compatability snafus that way. we don't have to say "in zuul v3.7, pushes to refs/notes now emit a 'note' event *instead of* a 'push' event) | 16:57 |
| mordred | so it's just mostly my twitchy "exposing the guts is powerful, but potentially hard for new users to undestand" | 16:57 |
| jeblair | cost == cost of sending 2 events. | 16:57 |
| mordred | jeblair: ++ | 16:57 |
| openstackgerrit | Merged openstack-infra/nodepool feature/zuulv3: Validate flavor specification in config https://review.openstack.org/451875 | 16:58 |
| *** Cibo_ has joined #zuul | 16:59 | |
| jeblair | mordred: yeah. i want cake and eating cake. :) [to seriousify that, we've benefited greatly from making things like this easy, but we also created 4 kinds of pipelines we never dreamed of without having to write any python code by exposing these things. so i like both.] | 16:59 |
| jlk | so to recap this, if I understand it right. | 17:00 |
| jlk | For Github, keep the event as "push" for github, and within push handle both tags/branches by way of regex, instead of synthesizing it. | 17:00 |
| jlk | At a later time, we'll do a proposal to synthesize some branch vs tag type events, that will cover all the drivers. | 17:01 |
| jeblair | jlk: yep (and that mostly means the github driver doesn't have to do anything smart with push right now) | 17:01 |
| jlk | A+ | 17:02 |
| jeblair | one more borderline bikeshed: | 17:02 |
| jeblair | we've been going with 'pull_request.opened' as the mechanical translation of a "pull_request" event with action="opened". | 17:03 |
| jeblair | strictly speaking, the zuul config language can handle "{event: pull_request, action: opened}" just fine. | 17:03 |
| jlk | hrm | 17:03 |
| jeblair | i wrote up style(C) in the etherpad to show what that would look like | 17:04 |
| jlk | right | 17:04 |
| jlk | DO we use dot notation anywhere else within layouts? | 17:05 |
| jlk | because if not, style C seems more "natural" | 17:05 |
| jeblair | nope, the dot notation is new | 17:05 |
| clarkb | +1 to verbose layout | 17:05 |
| jeblair | if you were to take a really literal view of how the gerrit trigger is configured (basically arbitrary matching of paremeters), it would actually look more like C. | 17:05 |
| *** harlowja has quit IRC | 17:06 | |
| jlk | yup, I lean toward C | 17:06 |
| jeblair | i'm okay with dot notation if folks want it and it feels more human-friendly. | 17:06 |
| jlk | You also had input about preventing comment on reporting | 17:07 |
| jeblair | but i'm also okay with C. C probably makes our "pull_request comment" lie slightly more of a lie (and maybe more confusing). | 17:07 |
| jlk | and I'd like to go through that with you if you're ready. | 17:07 |
| jeblair | mordred: ^ you proposed the dot notation -- i don't want ^ to slip by without you having a chance to scream. | 17:08 |
| jeblair | jlk: oh yes | 17:08 |
| jeblair | jlk: 444060, right? | 17:08 |
| jeblair | jlk: do you have any links to what a zuul comment in github looks like? | 17:09 |
| jlk | yes | 17:09 |
| jlk | oh I was saying ues to 444060 | 17:09 |
| jlk | I'll get to that in a moment, but I want to explain the reasoning. | 17:09 |
| jeblair | heh, figured :) | 17:09 |
| jeblair | ok | 17:09 |
| jlk | In Github, there is the concept of a "status" | 17:09 |
| jlk | https://github.com/BonnyCI/zuul/pull/47 | 17:10 |
| jlk | has such a status | 17:10 |
| jlk | check_github has failed, and we set the status to failure. | 17:10 |
| jlk | Github has traditionally used status on a commit hash to hold CI responses | 17:11 |
| jlk | and in fact, "branch protection" is a set of conditions that must be met before somebody can push the "merge" button on a PR. One such protection is that an appropriate status is set. | 17:11 |
| jlk | When a status is set, it does not cause an email from github to go out | 17:12 |
| jlk | whereas comments get emailed to everybody who's attached to the ... issue. | 17:12 |
| jlk | that gets noisy in a hurry, and people seem to not like it. | 17:12 |
| jlk | (one sec, dog needs out) | 17:12 |
| jeblair | so does the status show up as the little red X next to b6b5f9a above the comment? | 17:14 |
| jlk | In general, robot actions on github get treated different than human actions. | 17:14 |
| jlk | jeblair: yes, it shows up as the x. And for people with merge rights, it shows up in a summary too | 17:14 |
| jeblair | when you set a status, you also set a url; where is that used? | 17:15 |
| SpamapS | that's for the log output of the job | 17:15 |
| jlk | that red X is clickable | 17:15 |
| SpamapS | "Details" in the UI | 17:15 |
| jlk | that's where the URL comes into play | 17:15 |
| jeblair | ha, HCI affordance failure. :) | 17:15 |
| jeblair | it moused-over as "Failure" so i did not assume it was a link. :) | 17:16 |
| jlk | gotcha | 17:16 |
| jlk | I don't know if you see a summary down at the bottom of the PR of the reviews and statuses | 17:16 |
| jeblair | (i see the link at the bottom now though) | 17:16 |
| jlk | at least there it says "Details" | 17:16 |
| jeblair | no summary for anonymous me | 17:16 |
| mordred | jeblair: I'm perfectly fine with C | 17:16 |
| jlk | k | 17:16 |
| jlk | Github has 3 different ways to provide input | 17:17 |
| jlk | there's the status we're talking about, where CI robots put pending, success, fail. | 17:17 |
| jlk | there's commentary on the PR directly | 17:17 |
| jlk | there's commentary in the changes view of the PR (yes, it's different) | 17:17 |
| jlk | and there's a new system called "Reviews" (I lied there's 4) | 17:17 |
| jlk | We still need to gather data from users about where they'd like BonnyCI to report things. whether a comment on failure is appropriate, or if the status is enough. We're pretty sure they don't want a comment on start, and probably don't want a comment on success | 17:18 |
| jlk | https://github.com/BonnyCI/zuul/pull/40 is an example where a job failed | 17:19 |
| mordred | are the different places one can report things that could be exposed at pipeline config time? so that a user could say "please report statuses only to robot status" or "please also to make comments" ? | 17:19 |
| jlk | we comment on failure, providing the link | 17:20 |
| jeblair | jlk: https://github.com/BonnyCI/zuul/pull/47#issuecomment-296492291 looks like a failure comment, right? | 17:20 |
| jlk | mordred: as it stands in the patch set, yes. pipelines can influence whether you get status or comment or both or neither | 17:20 |
| mordred | so the strategy is currently "put pending, sucess and failure status in the robot place, and also on failure make a comment with links"? | 17:20 |
| jlk | jeblair: oh right, yes there was a failure there too. | 17:21 |
| jlk | mordred: that's BonnyCI's strategy | 17:21 |
| jlk | mostly because a "status" is attached to a given commit hash | 17:21 |
| jeblair | the only way to get an itemized report of the run is still with an actual comment though, right? | 17:21 |
| jlk | if somebody updates the PR with a wholly new commit hash, that failing status is lost | 17:21 |
| mordred | yah. so if you update the PR, the status would no longer be valid, right? | 17:21 |
| mordred | ah - that too | 17:21 |
| jlk | jeblair: that's correct. | 17:21 |
| *** Cibo has joined #zuul | 17:21 | |
| mordred | so you'd also lose job history without the failure comments | 17:22 |
| jlk | jeblair: unless | 17:22 |
| jlk | jeblair: unless we make the URL not go directly to the logs, but instead go to a summary page | 17:22 |
| jlk | mordred: correct. This is another github bonghit | 17:22 |
| mordred | for myself, I really like being able to get the content directly in a change/pr without having to go exit to another system | 17:23 |
| jlk | that summary page thing could be entirely a BonnyCI thing, since we can control the URL passed to the status. | 17:23 |
| jeblair | jlk: right. we don't have a way to generate a summary page at the moment (the closest thing we have is the link to the log index page, which it looks like you're doing. | 17:23 |
| jlk | mordred: indeed, and this is definitely an area where we need to gather input from a wide swath of users | 17:23 |
| *** Cibo_ has quit IRC | 17:23 | |
| mordred | jlk: ++ | 17:24 |
| jlk | I could totally see comment on success/failure being a way users decide they want to go | 17:24 |
| jlk | my HUNCH is that less people care about a summary of success and more care about a summary on failure | 17:24 |
| mordred | yup | 17:24 |
| jeblair | jlk: i think summary page generation would be useful for zuul in general, so if y'all write that, please upstream it. :) | 17:24 |
| mordred | that's my hunch too - and I'd argue is a great 'default' | 17:24 |
| jlk | jeblair: totes! | 17:24 |
| mordred | ut I could see a project deciding they did not want the failure comments | 17:25 |
| mordred | I think it's unlikely anyone would want the statuses not posted to the robot status thing | 17:25 |
| mordred | (well, except for thing-party-ci's where they don't have access to do so) | 17:25 |
| jlk | well... | 17:25 |
| jlk | status reporting is pretty wide open I think. | 17:25 |
| jeblair | so i do now see the need to disable comments. so how about we keep that feature, but default it to 'leave comments' so that novice zuul users get useful output. but it's easy to turn off. | 17:26 |
| jlk | oh I take that back | 17:26 |
| mordred | so the status just needs _someone_ to post success? or can you say for protected branches "please only trust statuses from XXX" | 17:26 |
| jlk | jeblair: I'm totally fine with that. | 17:26 |
| jlk | mordred: I was wrong. YOu can grant specifically teh right to set status | 17:27 |
| mordred | ok. cool | 17:27 |
| jlk | and you can also narrow down _which_ status is required for merging | 17:27 |
| jeblair | mordred: i'm 95% with you, except i can concieve of wanting to do 'pending' followed by 'success' even on failure (it closes out the action, but doesn't splat a red x on things. think of "nonvoting pipeline") | 17:27 |
| mordred | jeblair: yah. I could see that too | 17:27 |
| jlk | yeah it gets ugly because an _overall_ status is used for the UI display | 17:27 |
| jlk | all statuses pass, you get green. ANY status fails, you get red | 17:28 |
| SpamapS | status reporting requires write access | 17:28 |
| * SpamapS is a bit lagged out | 17:28 | |
| mordred | even if it's a non-binding status I guess? | 17:28 |
| jlk | whether or not that red will BLOCK the merge is a different storry | 17:28 |
| mordred | fantastic | 17:28 |
| jlk | mordred: I believe so, but I can't easily set up a test of that | 17:28 |
| jeblair | that's what prompted my other suggestion, which was change status to an enum rather than bool in zuul config. are there any problems with that? | 17:28 |
| mordred | no worries - mostly just learning - the answer isn't specifically germane | 17:28 |
| jlk | SpamapS: by default yes, but you can explicitly grant status set rights without merge rights. | 17:28 |
| SpamapS | jlk: oh that's handy | 17:29 |
| jlk | jeblair: can you sketch out your enum suggestion on the etherpad? | 17:29 |
| jeblair | yep | 17:30 |
| *** persia has quit IRC | 17:30 | |
| jeblair | jlk: at bottom | 17:31 |
| jlk | Okay, so in that case, the end user has to know which status states are allowed | 17:31 |
| jlk | but that's fine | 17:31 |
| jeblair | jlk: i think the current version of the patch just has it hard-coded anyway | 17:32 |
| jlk | it does | 17:32 |
| jlk | it takes the decision away, but it also means consumers don't have to learn the restrictions. | 17:32 |
| jlk | but, I'm not strongly tied to that. | 17:33 |
| jeblair | jlk: what restrictions? | 17:33 |
| jlk | I'm okay with an enum | 17:33 |
| jeblair | i mean, i know they may not have perms for a given status | 17:33 |
| jlk | jeblair: the Github API will only accept pending, success, or fail. | 17:33 |
| jlk | you can't do status: bonkers | 17:33 |
| jeblair | but the current version of the patch would fail if you said "status: true" but couldn't set success | 17:33 |
| jeblair | jlk: oh yeah | 17:33 |
| jeblair | jlk: we can validate the enums in zuul | 17:33 |
| jlk | cool | 17:34 |
| jeblair | (we just can't (easily) validate that they have permission) | 17:34 |
| *** Cibo_ has joined #zuul | 17:34 | |
| jlk | yeah the permissions don't come into play. Status perms are all or nothing. | 17:34 |
| jeblair | but that's true for many, many things in the zuul conf. | 17:34 |
| jlk | Anybody else have objections to using a (validated) enum for status? | 17:35 |
| jlk | the code for this is kind of wonky, because it all falls under "report", and the report action in github driver has to tease out what all reports should be done, calling out to other sub-functions. It's do-able, but only slightly awkward. | 17:36 |
| *** Cibo has quit IRC | 17:36 | |
| jeblair | jlk: yay abstraction layers! | 17:37 |
| jlk | alright, was there any other big things to discuss? | 17:38 |
| jeblair | jlk: i *think* that's all open ended questions i left masquerading as review comments..., yeah? | 17:38 |
| jlk | I think so | 17:38 |
| jlk | I have one thing that's not directly related, that I wanted to wait until after | 17:38 |
| jeblair | after like now, or after like after now? :) | 17:39 |
| jlk | after like now would work | 17:39 |
| jeblair | ok | 17:39 |
| jlk | So you've done work in a way that a pipeline could service for multiple drivers | 17:39 |
| jlk | or sources? | 17:39 |
| jlk | you took source out of the top pipeline stanza, and let there be a source reference for trigger/success/fail | 17:40 |
| jlk | (is it a configured source name, or is it a driver name?) | 17:40 |
| jeblair | jlk: yeah, pipelines don't have sources anymore. so a single pipeline can be triggered by different drivers, and changes from both drivers can end up in that pipeline and they should be happy. | 17:41 |
| jeblair | jlk: sources are attached to projects directly | 17:41 |
| jlk | I get confused because in the examples and tests we tend to name the source after the driver | 17:41 |
| jeblair | (so those examples on the etherpad where i put gerrit and github together under trigger were legit) | 17:41 |
| jlk | so it's hard to see which is a named source reference and which is a driver name reference | 17:41 |
| jlk | either way, the intent is mixed drivers, one pipeline | 17:42 |
| jeblair | yeah, and it doesn't help that this stuff is changing. :) | 17:42 |
| jeblair | lemme take a crack at it: | 17:42 |
| jeblair | a pipeline has *triggers* which are *connections* to remote servers implemented by *drivers*. a *trigger* may emit an *event* with an associated *change* for a *project* which is fetched from a *source* over a *connection*. | 17:44 |
| jlk | The reason I bring it up, is regardless of driver or source, mixing them in a pipeline breaks down at the require/reject level: https://review.openstack.org/#/c/449390/6/tests/fixtures/layouts/requirements-github.yaml | 17:44 |
| jlk | Unless we try to map all drivers features into gerritisms for require/reject, I think we'll wind up with incompatibilities | 17:45 |
| jlk | (or we come up with zuulisms to map everything to) | 17:46 |
| jeblair | jlk: yep, that's going to be muddled. | 17:46 |
| jlk | In the pipeline there, if I were to try to throw gerrit projects at that pipeline, it'd fail. | 17:46 |
| jlk | Would it make sense, in the future, to put require/reject into the same connection break down? | 17:46 |
| jlk | require: github: <something> require: gerrit: <something_else> | 17:47 |
| clarkb | that is what i was just going to sugget | 17:47 |
| jeblair | it's currently a fairly limited set of things you can require, and they are aspects of the Change, so it's not inconceivable that we could abstract them. | 17:47 |
| clarkb | and then only apply the requirements to the corresponding trigger | 17:47 |
| jeblair | clarkb: well, these requirements should be independent of the trigger, though they may not be independent of the underlying connection/driver | 17:48 |
| jlk | In github land, we would want to be able to require a status (see status discussion above), or a passing review from somebody with commit access (see discussion of inputs above), or a label being present, maybe even a comment being present. | 17:48 |
| clarkb | the requirements control when a trigger applies right? | 17:48 |
| jeblair | clarkb: (imagine a pipeline triggered only by IRC, but still has a "status: open" pipeline requirement) | 17:48 |
| jlk | https://github.com/BonnyCI/hoist/blob/master/roles/zuul/templates/etc/zuul/config/layout.yaml#L46 is our v2.5 layout | 17:49 |
| jeblair | clarkb: we have two very similar things -- trigger requirements and pipeline requirements. trigger requirements are easy, but pipeline requirements are, ideally, more independent | 17:49 |
| jlk | for our "gate" | 17:49 |
| jlk | and there is a reason why we list these (partially) as both a trigger req _and_ a pipeline req | 17:50 |
| jlk | Any time you have multiple requirements, you have to accept that they'll come in unordered | 17:51 |
| jlk | you're not going to get _both_ requirements in teh same event | 17:51 |
| jlk | so you need to allow yourself to "trigger" on any of the events that _could_ meet requirement, and then use pipeline requirements to see if all are met | 17:51 |
| jlk | unless, I Just had a light bulb moment | 17:51 |
| jlk | We have to _trigger_ on all the potential events. A trigger requirement, will that fetch details about the change itself (not just the data in the event) and would be able to see if the other requirements are already in place? | 17:52 |
| jeblair | jlk: in the gerrit driver we did a weird 'change-has-approval' trigger requirement, so even if you get 2 approvals in sequence, you can have both requirements in the trigger req | 17:52 |
| jlk | I.E. could I move my pipeline requirements into being trigger requirements? | 17:52 |
| jeblair | jlk: yeah, i think you just invented that :) | 17:52 |
| jlk | alright, I think that would side-step the question of what to do with multi-driver pipline requirements. | 17:53 |
| jeblair | jlk: "required_approvals" is i think what we called it | 17:53 |
| jlk | those have to be generic enough. | 17:53 |
| jeblair | jlk: (also spelled as 'require-approval') | 17:54 |
| *** Cibo_ has quit IRC | 17:54 | |
| jlk | jeblair: I think that's right. it was a little difficult to wrap my head around it, and I think I punted on trigger requirement at hte time. That was a mistake I think. I think these should have been implemented as trigger requirements instead of pipeline requirements. | 17:54 |
| *** Cibo has joined #zuul | 17:54 | |
| jeblair | jlk: oh, and actually, that's in the BaseFilter object, which is probably way on the wrong side of what should be an api layer. | 17:55 |
| jeblair | it's got some decidedly gerrit things in there | 17:55 |
| jlk | yeah | 17:55 |
| jlk | this was an area that needed discussion to get right | 17:56 |
| jlk | We plowed ahead with a brute force method, with knowledge that it'll likely get changed a lot to land in v3 | 17:56 |
| jlk | it's also where we had to start editing model.py more which felt wrong | 17:57 |
| * SpamapS is caught up and nodding but has nothing to add | 17:57 | |
| jlk | jeblair: now, if they were trigger requirements, the triggers we list in a pipline, can we have different triggers per connection using the same driver, or do all connections using the same driver have to share the same trigger configuration? | 17:59 |
| jeblair | in general, we should probably try to head to a point where the gerrit driver spits out a GerritChange(Change) and github driver emits GitHubPullRequest(Change). and the Change object implements an api for abstract pipeline requirements (which they can override). we're still going to need some kind of solution though for approvals, because that's just not going to abstract. so we'll need to separate them by driver in the config, or extend the ... | 17:59 |
| jeblair | ... language to boolean-or sets of pipeline requirements. | 17:59 |
| jlk | say I have a github and a github_enterprise connection. Both use github driver. Could I have one pipeline that requires one thing for github and something different for github_enterprise ? | 18:00 |
| jeblair | jlk: yes -- the word 'gerrit' and 'github' that we conventionally use in pipeline triggers is actually the connection name. so "trigger: {github_enterprise: ..., github: ...}" is valid | 18:01 |
| jlk | Okay great. | 18:01 |
| jlk | okay I think I have enough to chew on today and into next week | 18:03 |
| jeblair | yay! this has been fun. thanks :) | 18:03 |
| jlk | thank you! | 18:03 |
| jeblair | 16:36 < clarkb> jeblair: re the security related change you posted. Would it be simpler to just not allow trusted repos to dep on untrusted repos? | 18:03 |
| jeblair | clarkb: re that ^ -- i think it would be too constraining to say that a playbook in a trusted repo can't use certain roles. | 18:05 |
| jeblair | clarkb: i mean, we might set up some infra role repos which otherwise don't really need to be in a trusted repo. but we may want to use them from project-config. | 18:05 |
| * jlk goes for a bike ride | 18:05 | |
| jeblair | clarkb: this should let us do that safely. | 18:06 |
| *** yolanda has quit IRC | 18:07 | |
| clarkb | jeblair: I'm just thinking that without any implementation details a trusted entity depending on an untrusted entity transitively makes the entire setup untrusted | 18:08 |
| clarkb | we're trying to work around that here or we could possibly just avoid the issue entirely? | 18:08 |
| jeblair | clarkb: well, now i think we've gone and gotten ourselves in trouble with our new 'friendly' terminology of trusted/untrusted. :) | 18:09 |
| jeblair | clarkb: the author of the playbook in the "trusted" repo *did* trust the maintainers of the role they chose to depend on enough to add that role as a dependency | 18:09 |
| jeblair | clarkb: the only thing that would be unexpected to them, is if someone convinced zuul to run their trusted playbook with un-merged code to that role. | 18:10 |
| clarkb | I see | 18:10 |
| jeblair | because that's trusting "random person on the internet" versus "the authors of this role" :) | 18:10 |
| * SpamapS should take a break from o_O'ing at gear and how to elegantly provide a text facade :-P | 18:10 | |
| jeblair | clarkb: so the super-short version of the change is "when we run an trusted playbook, we don't use speculative merging" | 18:12 |
| jeblair | * for roles (it was already the case for the playbooks themselves) | 18:12 |
| *** yolanda has joined #zuul | 18:16 | |
| openstackgerrit | Merged openstack-infra/nodepool feature/zuulv3: Add support for specifying key-name per label https://review.openstack.org/455464 | 18:30 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Move makemergeritem into model https://review.openstack.org/460714 | 18:38 |
| *** harlowja has joined #zuul | 18:38 | |
| *** _ari_ is now known as _ari_|gone | 18:57 | |
| *** harlowja has quit IRC | 21:17 | |
| *** harlowja has joined #zuul | 21:17 | |
| *** dmsimard|cavern is now known as dmsimard | 21:20 | |
| *** harlowja has quit IRC | 21:36 | |
| *** cmurphy has quit IRC | 21:59 | |
| jlk | jeblair: still around? Looking at implementing things from our meeting today. A pull request event could have one of many actions. Should we support actions as a list of things, or should we require adding a new event: pull_request block for each action? | 22:04 |
| *** cmurphy has joined #zuul | 22:05 | |
| jeblair | jlk: most things like that in zuul silently handle both. there are some "as_list" and "to_list" helper methods that coerce single-items into lists. | 22:05 |
| jeblair | jlk: so maybe do that? | 22:05 |
| jlk | yeah trying to think of how that would look in the YAML | 22:05 |
| jlk | taking a stab in the etherpad | 22:05 |
| jlk | See style D line 43? https://etherpad.openstack.org/p/7HFyMRf519 | 22:06 |
| SpamapS | we need to fix testrepository so it uses a db type that exists in python 2 and 3 | 22:08 |
| SpamapS | No module named gdbm | 22:08 |
| SpamapS | silly testr | 22:08 |
| jlk | god damn bowel movement ? | 22:08 |
| SpamapS | jlk: we're not supposed to drink until 3:30 man. ;) | 22:09 |
| jlk | eh, I rode 20+ miles, I should be able to drink | 22:09 |
| SpamapS | jeblair: https://review.openstack.org/461172 redoes the TextJob and TextWorker to be simpler and use inheritance. Probably needs some docstrings. | 22:11 |
| SpamapS | jeblair: I did it as a new change becuase the old one was a huge mess. | 22:11 |
| jeblair | jlk: yep. style d matches existing similar things to my recollection. | 22:13 |
| jeblair | SpamapS: cool! | 22:14 |
| jeblair | SpamapS: i'm trying to unload some gnarly git stuff from my brain right now, so will probably look at it monday | 22:14 |
| clarkb | SpamapS: the gdbm thing is independent of python2 and python3 | 22:15 |
| clarkb | SpamapS: also it works if you do python3 before python2 | 22:16 |
| clarkb | SpamapS: I have to install a separate package to get gdbm locally | 22:16 |
| SpamapS | jeblair: no rush. Thanks. | 22:17 |
| SpamapS | I may polish it a bit before then too. | 22:17 |
| SpamapS | clarkb: the 3 before 4 thing doesn't work with 3.5 | 22:17 |
| SpamapS | err | 22:17 |
| SpamapS | clarkb: the 3 before 2 thing | 22:17 |
| SpamapS | 3.5 dropped the thing that was backward compatible with 2 | 22:17 |
| SpamapS | can probably just put gdbm in test-requirements | 22:18 |
| clarkb | fun | 22:18 |
| SpamapS | but really, seems like testr should just put gdbm in its reqs, and use that. | 22:19 |
| clarkb | the problem is its part of the python build itself aiui | 22:19 |
| clarkb | not something pip installable | 22:19 |
| mordred | yah. it's all about the python compilation | 22:19 |
| jlk | eww | 22:20 |
| mordred | ya | 22:20 |
| clarkb | you could put it in bindep though | 22:20 |
| clarkb | or maybe we just need tsetr to maintain two sets of dbs | 22:20 |
| SpamapS | looks like it uses "anydbm" | 22:22 |
| SpamapS | but that's kind of a flawed thing | 22:23 |
| SpamapS | since it doesn't mean you'll have the same dbm across interpreters | 22:23 |
| SpamapS | I wonder how badly dumbdbm performs.. at least it is consistent across 2/3 | 22:24 |
| SpamapS | what a weird problem | 22:27 |
| mordred | SpamapS: yah - who would have thought? | 22:27 |
| clarkb | it would be fine if we just split by python major version I think | 22:27 |
| SpamapS | clarkb: indeed, that may be easier than figuring out how to make one that works for all | 22:28 |
| SpamapS | clarkb: you actually probably have to go by something unique to the build tho... as mordred said.. one could compile custom python that breaks it | 22:29 |
| *** jkilpatr has quit IRC | 22:30 | |
| clarkb | ya though that seems less of a problem with tox at least | 22:32 |
| clarkb | since tox is going to make a virtualenv that will stick around for some time | 22:32 |
| SpamapS | here's a thought | 22:32 |
| SpamapS | sqlite | 22:32 |
| clarkb | ya I think mtreinish has considered a fork that does that | 22:33 |
| SpamapS | do we have to fork it? AFAIK, the owners are all inactive, or lifeless | 22:33 |
| clarkb | mtreinish would know better than me. The impression I got was yes no one reviews code and no one else can get commit access | 22:33 |
| SpamapS | ah yeah | 22:34 |
| * SpamapS opens a window to let out the yak smell | 22:34 | |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Store initial repo state in the merger https://review.openstack.org/461176 | 22:55 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Use previously stored repo state on executor https://review.openstack.org/461177 | 22:55 |
| jeblair | whew! that's today's second fix for an 'unsuitable for production' issue. it feels really good to exorcise that from my brain. | 22:57 |
| mordred | jeblair: brian exorcisms are the best | 22:59 |
| mordred | gah | 22:59 |
| mordred | brain | 22:59 |
| mordred | jeblair: first patch no likey | 22:59 |
| jeblair | i guess that's what i get for running flake8 *before* 'git commit -p' and manually editing the resulting diff | 23:00 |
| SpamapS | mordred: I'm Brian, and so's my wife | 23:01 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Store initial repo state in the merger https://review.openstack.org/461176 | 23:02 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Use previously stored repo state on executor https://review.openstack.org/461177 | 23:02 |
| jeblair | man, zuul and jenkins are just ganging up on me | 23:02 |
| mordred | they do that | 23:03 |
| mordred | jeblair: those both actually make complete sense | 23:06 |
| openstackgerrit | Jesse Keating proposed openstack-infra/zuul feature/zuulv3: Support GitHub PR webhooks https://review.openstack.org/439834 | 23:24 |
| jeblair | mordred: \o/ | 23:25 |
| jlk | egg | 23:25 |
| jlk | that got the wrong topic, which I'll fix in the larger rebase | 23:25 |
| jlk | but | 23:25 |
| jlk | jeblair: does that look like what you'd want to see? | 23:25 |
| jeblair | jlk: after a quick scan, yep! | 23:28 |
| jlk | cool. Tests pass locally. I'll await feedback in review before continuing on with the stuffs. | 23:28 |
| jlk | as an aside, is there an easy way to change the topic of a review, without needing to change the code of the review? | 23:28 |
| jeblair | jlk: i'm not sure validate_conf is used anywhere | 23:29 |
| jeblair | jlk: yes. | 23:30 |
| jlk | oops, I lifted that from the gerrit driver :) | 23:30 |
| jeblair | jlk: yeah, looks like it's dead code there too | 23:30 |
| jlk | neat! | 23:30 |
| *** harlowja has joined #zuul | 23:30 | |
| jeblair | jlk: i have changed the topic. i'm sure there's some way to do that it the webui, but it's been years since i've used it. it's C-t in gertty. | 23:31 |
| jlk | hahah | 23:31 |
| jlk | cheers, I'm going to go earn a beer by mowing the lawn! | 23:31 |
| jeblair | you and fungi and | 23:31 |
| jeblair | clarkb have almost convinced me to mow my lawn | 23:32 |
| jeblair | but i think i'm just going to sit and watch it grow instead | 23:32 |
| clarkb | I have to weed mine first | 23:32 |
| clarkb | then mow it | 23:32 |
| clarkb | then wonder why I got a place with a yard | 23:32 |
| jeblair | clarkb: kids can mow lawns starting at 6 months, rightL | 23:32 |
| jeblair | ? | 23:32 |
| clarkb | jeblair: I wish | 23:32 |
| clarkb | they "help" with the weeding though | 23:33 |
| jlk | strap them to the remote control lawn mower and sure | 23:33 |
| clarkb | then they blow the dandelion seeds everywhere | 23:33 |
| jlk | my 10 yo started mowing some this past summer. Right now he gets dog poop duty (hehehe, duty). | 23:33 |
| clarkb | also all the rain makes yardwork a giant mess | 23:33 |
| fungi | yup, been having to time it between storms | 23:34 |
| * SpamapS has a grass-free yard. | 23:34 | |
| fungi | with enough time to sort of dry back out | 23:34 |
| jlk | yeah, that's a big reason why I'm running out to do it now. This time of the year you gotta mow whenever i'ts dry, no matter the time. | 23:34 |
| SpamapS | succulents and palms and moss :) | 23:34 |
| jeblair | fungi: don't the storms just rip your lawn away? | 23:34 |
| clarkb | SpamapS: I'm encouraging the succulents | 23:34 |
| clarkb | SpamapS: they are neat plants | 23:34 |
| fungi | SpamapS: soon so shall i. just need to keep the grass^H^H^H^H^Hweeds short enough to avoid town ordinance violations until i can relandscape | 23:35 |
| fungi | jeblair: not often enough i'm afraid | 23:35 |
| clarkb | fungi: oh thats my other problem. I live in the county which means I basically have zero rules. Which means motivation at times is very low | 23:35 |
| fungi | SpamapS: but yeah, palms, ferns, bromeliads, succulents, cacti and other drought-tolerant flora | 23:36 |
| SpamapS | I also have some rather unruly rosemary bushes | 23:36 |
| fungi | soon to be rosemary trees | 23:36 |
| clarkb | you just need to cook with rosemary every day | 23:36 |
| clarkb | I just planted rosemary with ^ in mind | 23:37 |
| SpamapS | I do scrape rosemary off them when I need some :) | 23:37 |
| SpamapS | fantastic in turkey | 23:37 |
| fungi | and in bread! | 23:38 |
| *** harlowja has quit IRC | 23:55 | |
| *** jkilpatr has joined #zuul | 23:56 | |
| *** harlowja has joined #zuul | 23:59 | |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!