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 <[email protected]> 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 <[email protected]> > 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 <[email protected]> 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 <[email protected]> >>> 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é <[email protected]> >>>> 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 <[email protected] >>>>> > <mailto:[email protected]>> 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é >>>>> [email protected] >>>>> http://blog.nanthrax.net >>>>> Talend - http://www.talend.com >>>>> >>>>
