Unless there is a strenuous objection, I think we should roll forwards with a surgical "revert" of just the relevant logic, as in https://github.com/apache/beam/pull/1901
On Wed, Feb 1, 2017 at 5:22 PM, Kenneth Knowles <k...@google.com> wrote: > The revert is actually quite complex internally. I think there might be a > ~5 line roll forward, since the lost functionality was pretty trivial. I'll > give it a quick try, as it will be worth it if it works out. > > On Tue, Jan 31, 2017 at 1:38 PM, Aljoscha Krettek <aljos...@apache.org> > wrote: > >> I opened this PR with three revert commits: >> https://github.com/apache/beam/pull/1883 >> >> I also started PostCommit runs for this: >> - >> https://builds.apache.org/view/Beam/job/beam_PostCommit_Java >> _MavenInstall/2486/ >> - >> https://builds.apache.org/view/Beam/job/beam_PostCommit_Java >> _RunnableOnService_Flink/1493/ >> - >> https://builds.apache.org/view/Beam/job/beam_PostCommit_Java >> _RunnableOnService_Spark/803/ >> - >> https://builds.apache.org/view/Beam/job/beam_PostCommit_Java >> _RunnableOnService_Apex/ >> (still >> waiting in queue as of writing) >> - >> https://builds.apache.org/view/Beam/job/beam_PostCommit_Java >> _RunnableOnService_Dataflow/ >> (still >> waiting in queue as of writing) >> >> I think the MavenInstall hooks fail because the (Google-internal) Dataflow >> Runner Harness doesn't work with the changed code, though I'm only >> guessing >> here. >> >> >> On Tue, 31 Jan 2017 at 21:26 Aljoscha Krettek <aljos...@apache.org> >> wrote: >> >> Agreed, since it's a regression. Let's hope that the transitive closure of >> "revert those two commits" doesn't get to large. >> >> I'll checkout the release-0.5.0 branch and see where we get with >> reverting. >> >> On Tue, 31 Jan 2017 at 19:28 Kenneth Knowles <k...@google.com.invalid> >> wrote: >> >> I agree. -1 and let's do the smartest thing to undo the regression. >> >> Those two commits are not sufficient to restore late data dropping. You'll >> also need to revert the switch of the Flink runner to use new DoFn, maybe >> more. >> >> On Tue, Jan 31, 2017 at 10:21 AM, Jean-Baptiste Onofré <j...@nanthrax.net> >> wrote: >> >> > Basically, my question is: is it a regression ? If yes, definitely a -1 >> > and we should cancel the release. >> > >> > Correct me if I'm wrong, but the commits in the >> LateDataDroppingDoFnRunner >> > introduced a regression. So, I would cancel this vote and revert the two >> > commits for RC2. >> > >> > WDYT ? >> > >> > Regards >> > JB >> > >> > >> > On 01/31/2017 07:13 PM, Dan Halperin wrote: >> > >> >> Should we revert the CLs that lost the functionality? I'd really not >> like >> >> to ship a release with such a functional regression.... >> >> >> >> On Tue, Jan 31, 2017 at 10:07 AM, Jean-Baptiste Onofré < >> j...@nanthrax.net> >> >> wrote: >> >> >> >> Fair enough. Let's do that. >> >>> >> >>> Thanks ! >> >>> >> >>> Regards >> >>> JB >> >>> >> >>> >> >>> On 01/31/2017 06:58 PM, Aljoscha Krettek wrote: >> >>> >> >>> I'm not sure. Poperly fixing this will take some time, especially >> since >> >>>> we >> >>>> have to add tests to prevent breakage from happening in the future. >> >>>> Plus, >> >>>> if my analysis is correct other runners might also not have proper >> late >> >>>> data dropping and it's fine to have a release with some missing >> >>>> features. >> >>>> (There's more besides dropping.) >> >>>> >> >>>> I think we should go ahead and fix for 0.6. >> >>>> >> >>>> On Tue, Jan 31, 2017, 18:23 Jean-Baptiste Onofré <j...@nanthrax.net> >> >>>> wrote: >> >>>> >> >>>> Hi Aljoscha, >> >>>> >> >>>>> >> >>>>> so you propose to cancel this vote to prepare a RC2 ? >> >>>>> >> >>>>> Regards >> >>>>> JB >> >>>>> >> >>>>> On 01/31/2017 05:06 PM, Aljoscha Krettek wrote: >> >>>>> >> >>>>> It's not just an issue with the Flink Runner, if I'm not mistaken. >> >>>>>> >> >>>>>> Flink had late-data dropping via the LateDataDroppingDoFnRunner >> (which >> >>>>>> >> >>>>>> got >> >>>>> >> >>>>> "disabled" by the two commits I mention in the issue) while I think >> >>>>>> that >> >>>>>> the Apex and Spark Runners might not have had dropping in the first >> >>>>>> >> >>>>>> place. >> >>>>> >> >>>>> (Not sure about this last part.) >> >>>>>> >> >>>>>> As I now wrote to the issue I think this could be a blocker because >> we >> >>>>>> don't have the correct output in some cases. >> >>>>>> >> >>>>>> On Tue, 31 Jan 2017 at 02:16 Davor Bonaci <da...@apache.org> >> wrote: >> >>>>>> >> >>>>>> It looks good to me, but let's hear Aljoscha's opinion on >> BEAM-1346. >> >>>>>> >> >>>>>>> >> >>>>>>> A passing suite of Jenkins jobs: >> >>>>>>> * https://builds.apache.org/job/beam_PreCommit_Java_MavenInsta >> >>>>>>> ll/6870/ >> >>>>>>> * https://builds.apache.org/job/beam_PostCommit_Java_MavenInst >> >>>>>>> all/2474/ >> >>>>>>> * >> >>>>>>> >> >>>>>>> >> >>>>>>> https://builds.apache.org/job/beam_PostCommit_Java_RunnableO >> >>>>>>> >> >>>>>> nService_Apex/336/ >> >>>>> >> >>>>> * >> >>>>>> >> >>>>>>> >> >>>>>>> >> >>>>>>> https://builds.apache.org/job/beam_PostCommit_Java_RunnableO >> >>>>>>> >> >>>>>> nService_Flink/1470/ >> >>>>> >> >>>>> * >> >>>>>> >> >>>>>>> >> >>>>>>> >> >>>>>>> https://builds.apache.org/job/beam_PostCommit_Java_RunnableO >> >>>>>>> >> >>>>>> nService_Spark/786/ >> >>>>> >> >>>>> * >> >>>>>> >> >>>>>>> >> >>>>>>> >> >>>>>>> https://builds.apache.org/job/beam_PostCommit_Java_RunnableO >> >>>>>>> >> >>>>>> nService_Dataflow/2130/ >> >>>>> >> >>>>> >> >>>>>> On Mon, Jan 30, 2017 at 4:40 PM, Dan Halperin <dhalp...@apache.org >> > >> >>>>>>> >> >>>>>>> wrote: >> >>>>>> >> >>>>> >> >>>>> >> >>>>>> I am worried about https://issues.apache.org/jira/browse/BEAM-1346 >> >>>>>>> for >> >>>>>>> >> >>>>>>>> >> >>>>>>>> RC1 >> >>>>>>> >> >>>>>>> and would at least wait for resolution there before proceeding. >> >>>>>>>> >> >>>>>>>> On Mon, Jan 30, 2017 at 3:48 AM, Jean-Baptiste Onofré < >> >>>>>>>> j...@nanthrax.net >> >>>>>>>> >> >>>>>>>> >> >>>>>>> wrote: >> >>>>>> >> >>>>>>> >> >>>>>>>> Good catch for the PPMC, I'm upgrading the email template in the >> >>>>>>>> >> >>>>>>>>> >> >>>>>>>>> release >> >>>>>>>> >> >>>>>>> >> >>>>>>> guide (it was a copy/paste). >> >>>>>>>> >> >>>>>>>>> >> >>>>>>>>> Regards >> >>>>>>>>> JB >> >>>>>>>>> >> >>>>>>>>> >> >>>>>>>>> On 01/30/2017 11:50 AM, Sergio Fernández wrote: >> >>>>>>>>> >> >>>>>>>>> +1 (non-binding) >> >>>>>>>>> >> >>>>>>>>>> >> >>>>>>>>>> So far I've successfully checked: >> >>>>>>>>>> * signatures and digests >> >>>>>>>>>> * source releases file layouts >> >>>>>>>>>> * matched git tags and commit ids >> >>>>>>>>>> * incubator suffix and disclaimer >> >>>>>>>>>> * NOTICE and LICENSE files >> >>>>>>>>>> * license headers >> >>>>>>>>>> * clean build (Java 1.8.0_91, Maven 3.3.9, Debian amd64) >> >>>>>>>>>> >> >>>>>>>>>> Two minor comments that do not block the release: >> >>>>>>>>>> * Usually I like to see the commit id referencing the rc, since >> >>>>>>>>>> git >> >>>>>>>>>> >> >>>>>>>>>> tags >> >>>>>>>>> >> >>>>>>>> >> >>>>>>> can be changed. >> >>>>>>>> >> >>>>>>>>> * Just a formality, "PPMC" is not committee that plays a role >> >>>>>>>>>> >> >>>>>>>>>> anymore, >> >>>>>>>>> >> >>>>>>>> >> >>>>> you're a PMC now ;-) >> >>>>>> >> >>>>>>> >> >>>>>>>>>> >> >>>>>>>>>> >> >>>>>>>>>> On Fri, Jan 27, 2017 at 9:55 PM, Jean-Baptiste Onofré < >> >>>>>>>>>> >> >>>>>>>>>> j...@nanthrax.net> >> >>>>>>>>> >> >>>>>>>> >> >>>>>>> wrote: >> >>>>>>>> >> >>>>>>>>> >> >>>>>>>>>> Hi everyone, >> >>>>>>>>>> >> >>>>>>>>>> >> >>>>>>>>>>> Please review and vote on the release candidate #1 for the >> >>>>>>>>>>> version >> >>>>>>>>>>> >> >>>>>>>>>>> 0.5.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 C8282E76 [3], >> >>>>>>>> >> >>>>>>>>> * all artifacts to be deployed to the Maven Central Repository >> [4], >> >>>>>>>>>>> * source code tag "v0.5.0-RC1" [5], >> >>>>>>>>>>> * website pull request listing the release and publishing the >> API >> >>>>>>>>>>> reference manual [6]. >> >>>>>>>>>>> >> >>>>>>>>>>> The vote will be open for at least 72 hours. It is adopted by >> >>>>>>>>>>> >> >>>>>>>>>>> majority >> >>>>>>>>>> >> >>>>>>>>> >> >>>>>>> approval, with at least 3 PPMC affirmative votes. >> >>>>>>>> >> >>>>>>>>> >> >>>>>>>>>>> Thanks, >> >>>>>>>>>>> JB >> >>>>>>>>>>> >> >>>>>>>>>>> [1] https://issues.apache.org/jira >> /secure/ReleaseNote.jspa?proje >> >>>>>>>>>>> ctId=12319527&version=12338859 >> >>>>>>>>>>> [2] https://dist.apache.org/repos/dist/dev/beam/0.5.0/ >> >>>>>>>>>>> [3] https://dist.apache.org/repos/dist/release/beam/KEYS >> >>>>>>>>>>> [4] https://repository.apache.org/ >> content/repositories/orgapache >> >>>>>>>>>>> beam-1010/ >> >>>>>>>>>>> [5] https://git-wip-us.apache.org/ >> repos/asf?p=beam.git;a=tag;h=r >> >>>>>>>>>>> efs/tags/v0.5.0-RC1 >> >>>>>>>>>>> [6] https://github.com/apache/beam-site/pull/132 >> >>>>>>>>>>> >> >>>>>>>>>>> >> >>>>>>>>>>> >> >>>>>>>>>>> >> >>>>>>>>>> >> >>>>>>>>>> -- >> >>>>>>>>>> >> >>>>>>>>> Jean-Baptiste Onofré >> >>>>>>>>> jbono...@apache.org >> >>>>>>>>> http://blog.nanthrax.net >> >>>>>>>>> Talend - http://www.talend.com >> >>>>>>>>> >> >>>>>>>>> >> >>>>>>>>> >> >>>>>>>> >> >>>>>>> >> >>>>>> -- >> >>>>> Jean-Baptiste Onofré >> >>>>> jbono...@apache.org >> >>>>> http://blog.nanthrax.net >> >>>>> Talend - http://www.talend.com >> >>>>> >> >>>>> >> >>>>> >> >>>> -- >> >>> Jean-Baptiste Onofré >> >>> jbono...@apache.org >> >>> http://blog.nanthrax.net >> >>> Talend - http://www.talend.com >> >>> >> >>> >> >> >> > -- >> > Jean-Baptiste Onofré >> > jbono...@apache.org >> > http://blog.nanthrax.net >> > Talend - http://www.talend.com >> > >> > >
