Re: Precommits broken?

2018-06-14 Thread Scott Wegner
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  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  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  wrote:
>>
>>> That seems like a pretty good interim solution.
>>>
>>> On Thu, Jun 14, 2018 at 9:53 AM Andrew Pilloud 
>>> 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  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 
> 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 
>> 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 
>>> 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 
 wrote:

> Precommit filter is a really coool 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 
>> wrote:
>>
>>> Probably due to the precommit filter applied in #5611
>>> 

Re: Precommits broken?

2018-06-14 Thread Udi Meiri
+1 for separate jobs if it gets us faster to pre-commit filtering

On Thu, Jun 14, 2018 at 11:22 AM Kenneth Knowles  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  wrote:
>
>> That seems like a pretty good interim solution.
>>
>> On Thu, Jun 14, 2018 at 9:53 AM Andrew Pilloud 
>> 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  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 
 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 
> 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 
>> 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  wrote:
>>>
 Precommit filter is a really coool optimization!

 -Rui

 On Wed, Jun 13, 2018 at 5:21 PM Andrew Pilloud 
 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 
> wrote:
>
>> Probably due to the precommit filter applied in #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 

Re: Precommits broken?

2018-06-14 Thread Kenneth Knowles
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  wrote:

> That seems like a pretty good interim solution.
>
> On Thu, Jun 14, 2018 at 9:53 AM Andrew Pilloud 
> 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  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 
>>> 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 
 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 
> 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  wrote:
>>
>>> Precommit filter is a really coool optimization!
>>>
>>> -Rui
>>>
>>> On Wed, Jun 13, 2018 at 5:21 PM Andrew Pilloud 
>>> 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 
 wrote:

> Probably due to the precommit filter applied in #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?

Re: Precommits broken?

2018-06-14 Thread Andrew Pilloud
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  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 
> 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 
>> 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 
>>> 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  wrote:

> Precommit filter is a really coool optimization!
>
> -Rui
>
> On Wed, Jun 13, 2018 at 5:21 PM Andrew Pilloud 
> 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 
>> wrote:
>>
>>> Probably due to the precommit filter applied in #5611
>>> ?
>>>
>>> On Wed, Jun 13, 2018 at 5:02 PM Andrew Pilloud 
>>> 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 
 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
>>> 
>>>
>>


Re: Precommits broken?

2018-06-14 Thread Lukasz Cwik
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  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 
> 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  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  wrote:
>>>
 Precommit filter is a really coool optimization!

 -Rui

 On Wed, Jun 13, 2018 at 5:21 PM Andrew Pilloud 
 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  wrote:
>
>> Probably due to the precommit filter applied in #5611
>> ?
>>
>> On Wed, Jun 13, 2018 at 5:02 PM Andrew Pilloud 
>> 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 
>>> 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
>> 
>>
>


Re: Precommits broken?

2018-06-14 Thread Andrew Pilloud
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 
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  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  wrote:
>>
>>> Precommit filter is a really coool optimization!
>>>
>>> -Rui
>>>
>>> On Wed, Jun 13, 2018 at 5:21 PM Andrew Pilloud 
>>> 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  wrote:

> Probably due to the precommit filter applied in #5611
> ?
>
> On Wed, Jun 13, 2018 at 5:02 PM Andrew Pilloud 
> 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 
>> 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
> 
>


Re: Precommits broken?

2018-06-14 Thread Jason Kuster
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  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  wrote:
>
>> Precommit filter is a really coool optimization!
>>
>> -Rui
>>
>> On Wed, Jun 13, 2018 at 5:21 PM Andrew Pilloud 
>> 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  wrote:
>>>
 Probably due to the precommit filter applied in #5611
 ?

 On Wed, Jun 13, 2018 at 5:02 PM Andrew Pilloud 
 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 
> 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


Re: Precommits broken?

2018-06-13 Thread Scott Wegner
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  wrote:

> Precommit filter is a really coool optimization!
>
> -Rui
>
> On Wed, Jun 13, 2018 at 5:21 PM Andrew Pilloud 
> 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  wrote:
>>
>>> Probably due to the precommit filter applied in #5611
>>> ?
>>>
>>> On Wed, Jun 13, 2018 at 5:02 PM Andrew Pilloud 
>>> 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 
 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
>
>


Re: Precommits broken?

2018-06-13 Thread Rui Wang
Precommit filter is a really coool optimization!

-Rui

On Wed, Jun 13, 2018 at 5:21 PM Andrew Pilloud  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  wrote:
>
>> Probably due to the precommit filter applied in #5611
>> ?
>>
>> On Wed, Jun 13, 2018 at 5:02 PM Andrew Pilloud 
>> 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 
>>> 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




Re: Precommits broken?

2018-06-13 Thread Andrew Pilloud
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  wrote:

> Probably due to the precommit filter applied in #5611
> ?
>
> On Wed, Jun 13, 2018 at 5:02 PM Andrew Pilloud 
> 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 
>> 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
>>>
>>>


Re: Precommits broken?

2018-06-13 Thread Yifan Zou
Probably due to the precommit filter applied in #5611
?

On Wed, Jun 13, 2018 at 5:02 PM Andrew Pilloud  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 
> 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
>>
>>


Re: Precommits broken?

2018-06-13 Thread Andrew Pilloud
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  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
>
>


Precommits broken?

2018-06-13 Thread Andrew Pilloud
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