The issue is time from commit to merge, and without manual intervention, commits from other PRs aren't accounted for, if there's a lag between LGTM and merge.
On Fri, Sep 21, 2018, 1:52 PM Ahmet Altay <al...@google.com> wrote: > I will suggest a rollback in this case, and in general as a good practice > to unblock people. > > On Fri, Sep 21, 2018 at 1:02 PM, Charles Chen <c...@google.com> wrote: > >> Relatedly, https://github.com/apache/beam/pull/6151 also recently broke >> the build ( >> https://builds.apache.org/view/A-D/view/Beam/job/beam_PostCommit_Java_GradleBuild/1503/console) >> because the Precommits were very out of date when merged. >> > > Does not github change the test signals from green to yellow when new > commits are added? > > >> >> On Fri, Sep 21, 2018 at 12:50 PM Valentyn Tymofieiev <valen...@google.com> >> wrote: >> >>> The change https://github.com/apache/beam/pull/6424 was not deemed >>> particularly risky, and it's purpose was adding more tests to precommit >>> test suite. >>> There was a green Precommit signal on Jenkins, and I believe Postcommit >>> test suite (at the same time) wouldn't catch this. >>> >>> The reason the breakage was introduced is that >>> https://github.com/apache/beam/commit/7689f12db5 was committed after >>> the PR 6424 was reviewed, but before it was merged. A combination of both >>> introduced the breakage. >>> >>> Had we re-run the tests closer to the merge, we should have caught this. >>> Can we automatically re-run precommits tests at merge time, when some of >>> the files a PR is touching have changed since last precommit run? >>> >>> I suggest we proceed with https://github.com/apache/beam/pull/6464 or >>> revert https://github.com/apache/beam/pull/6424 in the mean time, >>> while we are iterating on the fix. >>> >>> On Fri, Sep 21, 2018 at 11:41 AM Charles Chen <c...@google.com> wrote: >>> >>>> Do we happen to know the root cause for why this wasn't caught during >>>> review / precommit? >>>> >>>> In the future, can we run manually run postcommits for risky changes >>>> like these? That is, trigger it by commenting "Run Python PostCommit"? >>>> >>>> On Fri, Sep 21, 2018 at 10:10 AM Pablo Estrada <pabl...@google.com> >>>> wrote: >>>> >>>>> Robbe has prepared a better fix on >>>>> https://github.com/apache/beam/pull/6465 >>>>> Hopefully that'll be the last of this breakage : ) >>>>> -P. >>>>> >>>>> On Fri, Sep 21, 2018 at 9:13 AM Jean-Baptiste Onofré <j...@nanthrax.net> >>>>> wrote: >>>>> >>>>>> By the way, it fails for me on my machine as well. >>>>>> >>>>>> Regards >>>>>> JB >>>>>> >>>>>> On 21/09/2018 18:10, Pablo Estrada wrote: >>>>>> > I investigated. This failure comes from the activation of >>>>>> > apache_beam.pipeline_test in Python 3 unit tests. >>>>>> > >>>>>> > I have https://github.com/apache/beam/pull/6464 out to fix this. >>>>>> > >>>>>> > In any case, it's very exciting that we have some unit tests >>>>>> running on >>>>>> > Py3 : ) >>>>>> > Best >>>>>> > -P. >>>>>> > >>>>>> > On Fri, Sep 21, 2018 at 6:40 AM Maximilian Michels <m...@apache.org >>>>>> > <mailto:m...@apache.org>> wrote: >>>>>> > >>>>>> > Hi, >>>>>> > >>>>>> > The Python PreCommit tests are currently broken. Is anybody >>>>>> looking >>>>>> > into >>>>>> > this? >>>>>> > >>>>>> > Example: >>>>>> > >>>>>> https://builds.apache.org/job/beam_PreCommit_Python_Commit/1308/#showFailuresLink >>>>>> > JIRA: https://issues.apache.org/jira/browse/BEAM-5458 >>>>>> > >>>>>> > I'm sure this was just an accident. No big deal but let's make >>>>>> sure >>>>>> > PreCommit passes when merging. A broken PreCommit means that we >>>>>> can't >>>>>> > merge any other changes with confidence. >>>>>> > >>>>>> > Thanks, >>>>>> > Max >>>>>> > >>>>>> >>>>>> -- >>>>>> Jean-Baptiste Onofré >>>>>> jbono...@apache.org >>>>>> http://blog.nanthrax.net >>>>>> Talend - http://www.talend.com >>>>>> >>>>> >
