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 >>> >>
