-1, as that PR does fix a critical bug. The fact that no unit test broke before was more a signal that our unit testing was deficient in this area.
My fix for the bug is pr/11226, which did include a unit test (which fails without the fix). However it appears that 11252 forked off just the main code files from my pr, and not the unit test. If we're recutting, we should include the unit test as well. On Mon, Apr 6, 2020 at 3:11 PM Rui Wang <ruw...@google.com> wrote: > I see. I will also leave the community to decide. > > With the unit tests in [1], the fix becomes sufficient (e.g. if the > community decides that the fix is critical, I will also need to include > those tests in the release). > > > [1] https://github.com/apache/beam/pull/11226 > > > -Rui > > > On Mon, Apr 6, 2020 at 3:05 PM Steve Niemitz <sniem...@apache.org> wrote: > >> My opinion doesn't matter much, since we're just going to cherry pick the >> fix into our fork anyways, but you're essentially proposing releasing a >> build that *WILL* cause data loss to anyone who uses processing time >> timers. >> >> I'll leave it up to the community to decide, but it seems like a pretty >> big bug. >> >> Also, fwiw, there is a PR open that adds a test for this [1], but it was >> never merged (it's been open for 12 days). >> >> [1] https://github.com/apache/beam/pull/11226 >> >> On Mon, Apr 6, 2020 at 5:52 PM Rui Wang <ruw...@google.com> wrote: >> >>> My opinion is, even though that commit was missing, no test/validation >>> gave a signal that something relevant was broken. Plus that fix didn't >>> include a test. >>> >>> I will hesitate to say such a fix is critical for a release, unless >>> there is something to test or validate it. >>> >>> >>> -Rui >>> >>> On Mon, Apr 6, 2020 at 2:46 PM Steve Niemitz <sniem...@apache.org> >>> wrote: >>> >>>> timers are essentially broken without it, so I'd say -1 >>>> >>>> On Mon, Apr 6, 2020 at 5:45 PM Rui Wang <ruw...@google.com> wrote: >>>> >>>>> ok so the source is consistent with the binary. What undecided is if >>>>> missing that commit is -1, or that can be marked as a known issue in >>>>> release note. >>>>> >>>>> >>>>> -Rui >>>>> >>>>> On Mon, Apr 6, 2020 at 2:38 PM Steve Niemitz <sniem...@apache.org> >>>>> wrote: >>>>> >>>>>> I can confirm that the artifact on maven central [1] does not have >>>>>> the change in it either, I disassembled it with javap. >>>>>> >>>>>> [1] >>>>>> https://repository.apache.org/content/repositories/orgapachebeam-1100/org/apache/beam/beam-runners-core-java/2.20.0/beam-runners-core-java-2.20.0.jar >>>>>> >>>>>> On Mon, Apr 6, 2020 at 5:28 PM Luke Cwik <lc...@google.com> wrote: >>>>>> >>>>>>> If the source doesn't represent the binaries, should that be an >>>>>>> automatic -1? >>>>>>> >>>>>>> On Mon, Apr 6, 2020 at 2:08 PM Valentyn Tymofieiev < >>>>>>> valen...@google.com> wrote: >>>>>>> >>>>>>>> On Mon, Apr 6, 2020 at 1:21 PM Robert Bradshaw <rober...@google.com> >>>>>>>> wrote: >>>>>>>> >>>>>>>>> Valentyn, do the container issues affect our external containers >>>>>>>>> as well? >>>>>>>>> >>>>>>>> >>>>>>>> No, external containers install Beam, so all Beam dependencies are >>>>>>>> also installed. >>>>>>>> >>>>>>>> Context (for others reading this): Currently built Dataflow Python >>>>>>>> containers don't install one of Beam 2.20.0 dependencies, which will be >>>>>>>> fixed. >>>>>>>> >>>>>>>> >>>>>>>>> I verified the signatures and sources, they all look good, except >>>>>>>>> that we're missing https://github.com/apache/beam/pull/11252 if >>>>>>>>> we were hoping to get that in. The wheel looks fine as well. >>>>>>>>> >>>>>>>>> On Mon, Apr 6, 2020 at 12:16 PM Rui Wang <ruw...@google.com> >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>>> A friendly ping to remind the vote for RC1 is pending. >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> -Rui >>>>>>>>>> >>>>>>>>>> On Mon, Apr 6, 2020 at 7:21 AM Péter Farkas <peter.far...@aliz.ai> >>>>>>>>>> wrote: >>>>>>>>>> >>>>>>>>>>> +1 - Validated only BEAM-9452 >>>>>>>>>>> <https://issues.apache.org/jira/browse/BEAM-9452> >>>>>>>>>>> >>>>>>>>>>> On Sat, 4 Apr 2020 at 00:22, Ahmet Altay <al...@google.com> >>>>>>>>>>> wrote: >>>>>>>>>>> >>>>>>>>>>>> +1 - Validated python quickstart examples. Thank you for >>>>>>>>>>>> preparing the RC. >>>>>>>>>>>> >>>>>>>>>>>> On Fri, Apr 3, 2020 at 12:25 PM Ismaël Mejía <ieme...@gmail.com> >>>>>>>>>>>> wrote: >>>>>>>>>>>> >>>>>>>>>>>>> Can somebody with windows please validate this one: >>>>>>>>>>>>> https://issues.apache.org/jira/browse/BEAM-9452 >>>>>>>>>>>>> >>>>>>>>>>>>> We really need to put some windows tests in place in the >>>>>>>>>>>>> future. Maybe we can >>>>>>>>>>>>> try github actions for this (but well the vote is not the >>>>>>>>>>>>> place to >>>>>>>>>>>>> discuss this). >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> I completely agree with you. I think we kind of already >>>>>>>>>>>> discussed this (https://issues.apache.org/jira/browse/BEAM-9388) >>>>>>>>>>>> but we did not get a chance to work on it. >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> On Fri, Apr 3, 2020 at 8:16 PM Rui Wang <ruw...@google.com> >>>>>>>>>>>>> wrote: >>>>>>>>>>>>> > >>>>>>>>>>>>> > Add Maven and Java versions that were used for building java >>>>>>>>>>>>> artifacts: >>>>>>>>>>>>> > maven: 3.6.2 >>>>>>>>>>>>> > java: 1.8.0_181 >>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>>> > -Rui >>>>>>>>>>>>> > >>>>>>>>>>>>> > On Thu, Apr 2, 2020 at 9:06 PM Rui Wang <ruw...@google.com> >>>>>>>>>>>>> wrote: >>>>>>>>>>>>> >> >>>>>>>>>>>>> >> Hi everyone, >>>>>>>>>>>>> >> Please review and vote on the release candidate #1 for the >>>>>>>>>>>>> version 1.20.0, as follows: >>>>>>>>>>>>> >> [ ] +1, Approve the release >>>>>>>>>>>>> >> [ ] -1, Do not approve the release (please provide specific >>>>>>>>>>>>> comments) >>>>>>>>>>>>> >> >>>>>>>>>>>>> >> >>>>>>>>>>>>> >> The complete staging area is available for your review, >>>>>>>>>>>>> which includes: >>>>>>>>>>>>> >> * JIRA release notes [1], >>>>>>>>>>>>> >> * the official Apache source release to be deployed to >>>>>>>>>>>>> dist.apache.org [2], which is signed with the key with >>>>>>>>>>>>> fingerprint 699A 22D2 D4F0 0AD3 957B 6A88 38B1 C6B4 25EB A67C >>>>>>>>>>>>> [3], >>>>>>>>>>>>> >> * all artifacts to be deployed to the Maven Central >>>>>>>>>>>>> Repository [4], >>>>>>>>>>>>> >> * source code tag "v1.20.0-RC1" [5], >>>>>>>>>>>>> >> * website pull request listing the release [6], publishing >>>>>>>>>>>>> the API reference manual [7], and the blog post [8]. >>>>>>>>>>>>> >> * Java artifacts were built with Maven MAVEN_VERSION and >>>>>>>>>>>>> OpenJDK/Oracle JDK JDK_VERSION. >>>>>>>>>>>>> >> TODO: do these versions matter, and are they stamped into >>>>>>>>>>>>> the artifacts? >>>>>>>>>>>>> >> * Python artifacts are deployed along with the source >>>>>>>>>>>>> release to the dist.apache.org [2]. >>>>>>>>>>>>> >> * Validation sheet with a tab for 2.20.0 release to help >>>>>>>>>>>>> with validation [9]. >>>>>>>>>>>>> >> * Docker images published to Docker Hub [10]. >>>>>>>>>>>>> >> >>>>>>>>>>>>> >> The vote will be open for at least 72 hours. It is adopted >>>>>>>>>>>>> by majority approval, with at least 3 PMC affirmative votes. >>>>>>>>>>>>> >> >>>>>>>>>>>>> >> Thanks, >>>>>>>>>>>>> >> Release Manager >>>>>>>>>>>>> >> >>>>>>>>>>>>> >> [1] >>>>>>>>>>>>> https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12319527&version=12346780 >>>>>>>>>>>>> >> [2] https://dist.apache.org/repos/dist/dev/beam/2.20.0/ >>>>>>>>>>>>> >> [3] https://dist.apache.org/repos/dist/release/beam/KEYS >>>>>>>>>>>>> >> [4] >>>>>>>>>>>>> https://repository.apache.org/content/repositories/orgapachebeam-1100/ >>>>>>>>>>>>> >> [5] https://github.com/apache/beam/tree/v2.20.0-RC1 >>>>>>>>>>>>> >> [6] https://github.com/apache/beam/pull/11285 >>>>>>>>>>>>> >> [7] https://github.com/apache/beam-site/pull/602 >>>>>>>>>>>>> >> [8] https://github.com/apache/beam/pull/11298 >>>>>>>>>>>>> >> [9] >>>>>>>>>>>>> https://docs.google.com/spreadsheets/d/1qk-N5vjXvbcEk68GjbkSZTR8AGqyNUM-oLFo_ZXBpJw/edit#gid=318600984 >>>>>>>>>>>>> >> [10] >>>>>>>>>>>>> https://hub.docker.com/search?q=apache%2Fbeam&type=image >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> -- >>>>>>>>>>> >>>>>>>>>>> Peter Farkas >>>>>>>>>>> >>>>>>>>>>> Lead Data Architect >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> www.aliz.ai >>>>>>>>>>> >>>>>>>>>>> LinkedIn <https://www.linkedin.com/company/alizcompany/>| Facebook >>>>>>>>>>> <https://www.facebook.com/aliztechnologies/>| Blog >>>>>>>>>>> <https://medium.com/@aliz_ai> >>>>>>>>>>> >>>>>>>>>>> <http://www.aliz.ai/> >>>>>>>>>>> >>>>>>>>>>
