Given that there have been no objections, I reenabled build comment triggers this morning in https://github.com/apache/beam/pull/22169. Hopefully the underlying issues are resolved, but if not we can quickly revert and engage Infra to restart Jenkins to restore the previous state.
Thanks, Danny On Fri, Jul 1, 2022 at 12:42 PM Danny McCormick <dannymccorm...@google.com> wrote: > Hey Yi, that's a good idea. Unfortunately, though, there is not a good way > to do it without modifying the plugin itself (not just the configuration). > Modifying the plugin is theoretically an option, but it is not easy - > finding a way to build/deploy it is hard, it doesn't build out of the box > right now, and it hasn't been touched in 2 years. If we're going to invest > the time, I'd vote we either use that time to move off of the plugin > entirely or move all our Jenkins infra to GitHub Actions (I'd vote to move > to GHA) - both of those require a lot of effort and are out of the scope of > this thread. > > It is worth being super clear here - anything we do that doesn't involve > extensively patching the plugin or moving off of it entirely is a band-aid. > As the community grows and our GitHub automation grows (as Yi pointed out) > this will continue to be an issue. This change should buy us a significant > amount of time though. > > Thanks, > Danny > > On Fri, Jul 1, 2022 at 11:12 AM Yi Hu via dev <dev@beam.apache.org> wrote: > >> Thanks for the investigation done and disabling the "admins verify patch" >> comment. Wondering if there is a way to early return the bot comments for >> ghprb (like blacklist)? Besides asf-ci there are also comments >> from github-actions (e.g. assign reviewer or "stopping review >> notifications"). In the future if we enable more github action items there >> might be more bot pr comments. >> >> Best, >> Yi >> >> On Fri, Jul 1, 2022 at 8:33 AM Danny McCormick via dev < >> dev@beam.apache.org> wrote: >> >>> Given the early consensus here, I tried disabling the "Can one of the >>> admins verify this patch?" messages, and verified that it worked. If you >>> disagree with that decision, please let me know - I expect that is the most >>> popular part of this proposal though :) >>> >>> The remaining questions are: >>> 1) Are we comfortable adding trusted repeat contributors to the Jenkins >>> allow-list? >>> 2) Are we ok trying to enable build triggers? >>> >>> Given that disabling the "Can one of the admins verify this patch?" >>> messages worked, I'm not sure that we need to do (1) - I think it would >>> help reduce load from the plugin a little bit, but is probably not as >>> necessary or helpful as what we've already done. I'm pretty comfortable >>> skipping that step. >>> >>> *If there are no objections, I'd like to try reenabling build comment >>> triggers next week.* Because I'll be offline for the first half of next >>> week for the American 4th of July holiday, I will plan on trying it next >>> Wednesday or Thursday if there are no objections. >>> >>> Thanks, >>> Danny >>> >>> On Thu, Jun 30, 2022 at 2:54 PM Pablo Estrada <pabl...@google.com> >>> wrote: >>> >>>> Agreed! I never know what to do about them : ) >>>> >>>> On Thu, Jun 30, 2022 at 10:58 AM Robert Burke <rob...@frantil.com> >>>> wrote: >>>> >>>>> +1 to get rid of the "admins very patch" comments if we can. They add >>>>> no value at all when commented in quadruplicate immediately after PR >>>>> creation. >>>>> >>>>> On Thu, Jun 30, 2022, 8:40 AM Danny McCormick < >>>>> dannymccorm...@google.com> wrote: >>>>> >>>>>> Hey everyone, I've been digging into the issues we've been having >>>>>> with Jenkins recently[1] which led to us disabling many of our build >>>>>> comment triggers (e.g. "Run <job name>" triggering a Jenkins job). Moving >>>>>> forward, I'd like to recommend that we: >>>>>> >>>>>> 1) Try to disable the "Can one of the admins verify this patch?" >>>>>> comments via the Jenkins plugin configuration. >>>>>> 2) Add trusted repeat contributors to the Jenkins allow-list. >>>>>> 3) Try re-enabling all build triggers. >>>>>> >>>>>> *Justification* >>>>>> >>>>>> Right now, around 33% of our PR comments are "Can one of the admins >>>>>> verify this patch?". Aside from being (IMO) very unhelpful and >>>>>> annoying, these are actually causing a significant amount of load on the >>>>>> ghprb plugin and indicate that these PRs are more expensive than those >>>>>> from >>>>>> allow-listed contributors. Since we believe that the ghprb plugin makes >>>>>> calls to GitHub that are roughly proportional to <the number of pr >>>>>> comments>X<the number of build triggers>, reducing our number of issue >>>>>> comments and the load per comment should give us enough breathing room to >>>>>> enable our triggers again. >>>>>> >>>>>> I wrote up a more thorough supporting doc here as well - - >>>>>> https://docs.google.com/document/d/15CILeNjNxCnbigSvxNq4eXPj6x6sn5DGdbTdWu55kCI/edit?usp=sharing >>>>>> >>>>>> *Disclaimer* >>>>>> >>>>>> It's really hard to empirically prove any of this after the fact - I >>>>>> think there's enough evidence to try it, but we should be ready to engage >>>>>> Infra to restart Jenkins and ready to revert any triggers we add. >>>>>> >>>>>> Thanks, >>>>>> Danny >>>>>> >>>>>> [1] Context on previous investigation here - >>>>>> https://docs.google.com/document/d/10qyUsvB_uVy5jftfTiwohlvN8Qwix5AuadssyoC4JsE/edit?usp=sharing >>>>>> >>>>>
