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