+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> >>>>>> >>>>>
smime.p7s
Description: S/MIME Cryptographic Signature
