Brilliant. That seems like a very clean solution that we can implement today. I'll get started; thanks for the idea Andrew!
On Thu, Jun 14, 2018 at 2:15 PM Udi Meiri <eh...@google.com> wrote: > +1 for separate jobs if it gets us faster to pre-commit filtering > > On Thu, Jun 14, 2018 at 11:22 AM Kenneth Knowles <k...@google.com> wrote: > >> I like Andrew's solution. Just totally separate jobs for automatic and >> manual. >> >> Kenn >> >> On Thu, Jun 14, 2018 at 9:56 AM Lukasz Cwik <lc...@google.com> wrote: >> >>> That seems like a pretty good interim solution. >>> >>> On Thu, Jun 14, 2018 at 9:53 AM Andrew Pilloud <apill...@google.com> >>> wrote: >>> >>>> If you always run one job for automated and another job for manual you >>>> wouldn't need to remember two trigger phrases. The automated jobs don't >>>> even need trigger phrases. As long as the status contexts are the same >>>> github users never have to know they are two separate jobs. >>>> >>>> Andrew >>>> >>>> On Thu, Jun 14, 2018 at 9:49 AM Lukasz Cwik <lc...@google.com> wrote: >>>> >>>>> I thought of that as well but would find it annoying that I would need >>>>> to remember two sets of triggers, the ones for the automated jobs and the >>>>> ones for the manual runs. If we re-use the same precommit trigger phrase, >>>>> we would get two runs (automated and manual) of effectively the same thing >>>>> for the jobs where the automated one wouldn't get filtered out. >>>>> >>>>> On Thu, Jun 14, 2018 at 9:46 AM Andrew Pilloud <apill...@google.com> >>>>> wrote: >>>>> >>>>>> 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> >>>>>>> >>>>>>
