Might there be a third option of creating a different jenkins job for PR change and manual triggers? It would clutter up the jenkins interface a bit, but they could both post status to the same commitStatusContext on Github, so no one would notice there.
Andrew On Wed, Jun 13, 2018 at 11:14 PM Jason Kuster <jasonkus...@google.com> wrote: > Having submitted a patch to the ghprb-plugin repo before, I think that > regretfully option (b) is probably the right decision here given that it's > unlikely to get accepted, merged, released, and to have Infra update the > plugin in under a week. > > On Wed, Jun 13, 2018 at 10:42 PM Scott Wegner <sweg...@google.com> wrote: > >> Indeed, I was going to send out an email about pre-commit filtering, but >> we've already found some kinks and may need to revert it. >> >> The change was submitted in PR#5611 [1] and enables Jenkins triggering to >> only run pre-commits based on modified files. However, Udi noticed that >> this also prevents manually running pre-commits on a PR with trigger >> phrases when your PR changes don't match the pre-commit include path [2]. >> This was blocking 2.5.0 release validation, so I have a PR out to revert >> the change [3]. >> >> I did some investigation and this is a deficiency in the Jenkins plugin >> used to trigger jobs on pull requests. I've filed a bug [4] and submitted a >> PR [5], but there's no guarantee that it'll get accepted or when it will be >> available. >> >> Question for others: we were hoping to enable pre-commit triggering as an >> optimization to decrease testing wait time and limit the impact of test >> flakiness [6]. But this bug in the plugin means we'd lose the ability to >> manually trigger pre-commits which aren't automatically run. One workaround >> would be to run the tests locally instead of on Jenkins, though that's >> clearly less desirable. Is this a blocker? >> >> Should we: >> (a) Keep pre-commit triggering enabled for now and hope the upstream >> patch gets accepted, or >> (b) Revert the pre-commit change and wait for the patch >> >> Thoughts? >> >> [1] https://github.com/apache/beam/pull/5611 >> [2] https://github.com/apache/beam/pull/5607#issuecomment-397080770 >> [3] https://github.com/apache/beam/pull/5638 >> [4] https://github.com/jenkinsci/ghprb-plugin/issues/678 >> [5] https://github.com/jenkinsci/ghprb-plugin/pull/680 >> [6] >> https://docs.google.com/document/d/1lfbMhdIyDzIaBTgc9OUByhSwR94kfOzS_ozwKWTVl5U/edit#bookmark=id.6j8bwxnbp7fr >> >> >> On Wed, Jun 13, 2018 at 10:03 PM Rui Wang <ruw...@google.com> wrote: >> >>> Precommit filter is a really coooooooooool optimization! >>> >>> -Rui >>> >>> On Wed, Jun 13, 2018 at 5:21 PM Andrew Pilloud <apill...@google.com> >>> wrote: >>> >>>> Ah, so this is intended and I didn't break anything? Cool! Sorry for >>>> the false alarm, looks like a great build optimization! >>>> >>>> Andrew >>>> >>>> On Wed, Jun 13, 2018 at 5:06 PM Yifan Zou <yifan...@google.com> wrote: >>>> >>>>> Probably due to the precommit filter applied in #5611 >>>>> <https://github.com/apache/beam/pull/5611>? >>>>> >>>>> On Wed, Jun 13, 2018 at 5:02 PM Andrew Pilloud <apill...@google.com> >>>>> wrote: >>>>> >>>>>> Looks like statuses got posted between me writing this email and >>>>>> sending it. Still wondering why the python and go jobs appear to be >>>>>> missing? >>>>>> >>>>>> Andrew >>>>>> >>>>>> On Wed, Jun 13, 2018 at 5:00 PM Andrew Pilloud <apill...@google.com> >>>>>> wrote: >>>>>> >>>>>>> Recent PRs don't appear to be running all the precommits, and >>>>>>> success status isn't being pushed to PRs. Anyone know what is going on? >>>>>>> >>>>>>> See: >>>>>>> https://github.com/apache/beam/pull/5592 >>>>>>> https://github.com/apache/beam/pull/5622 >>>>>>> >>>>>>> Andrew >>>>>>> >>>>>>> > > -- > ------- > Jason Kuster > Apache Beam / Google Cloud Dataflow > > See something? Say something. go/jasonkuster-feedback > <https://goto.google.com/jasonkuster-feedback> >
