ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alexey Goncharuk <alexey.goncha...@gmail.com>
Subject Re: Request for review : IGNITE-3303 Apache Flink Integration - Flink source
Date Mon, 01 Oct 2018 15:35:14 GMT
Hello Saikat,

I am ok with the prod code changes, but I am a bit confused with the
example being added to the tests folder. I think it should be either added
to the examples (not sure about the dependency though), or should not be
added at all. Also, I see that you added a new suite, has it been added to
a TC configuration?



пн, 1 окт. 2018 г. в 16:36, Nikolay Izhikov <nizhikov@apache.org>:

> Hello, Saikat.
>
> I have no objections to include this integration to 2.7 release.
> But, we should ask for a final review from Alex Goncharyuk.
>
> Alex, can you comment on this?
> Is this patch ready to be merged?
> Do you see any risks to include it to 2.7 release?
>
>
> В Вс, 30/09/2018 в 19:57 -0500, Saikat Maitra пишет:
> > Hi Alex, Nicolay
> >
> > As discussed with Andrew the changes looks good. Would it be ok to merge
> > this change to master considering the 2.7 release plan?
> >
> > Regards,
> > Saikat
> >
> > On Fri, Sep 28, 2018 at 7:15 PM Saikat Maitra <saikat.maitra@gmail.com>
> > wrote:
> >
> > > Thank you Andrew
> > >
> > > Regards,
> > > Saikat
> > >
> > > On Fri, Sep 28, 2018 at 7:00 PM Andrey Mashenkov <
> > > andrey.mashenkov@gmail.com> wrote:
> > >
> > > > Hi Saikat,
> > > >
> > > > Sorry for late answer. I've checked changes a day ago. Now, looks
> good.
> > > > Hope, it will be merged soon.
> > > >
> > > > Alex, would you please merge PR to master.
> > > >
> > > > сб, 29 сент. 2018 г., 2:29 Saikat Maitra <saikat.maitra@gmail.com>:
> > > >
> > > > > Hi Andrew,
> > > > >
> > > > > I have updated the changes.
> > > > >
> > > > > Can you please review and share feedback.
> > > > >
> > > > > Regards
> > > > > Saikat
> > > > >
> > > > > On Sat, Sep 22, 2018 at 2:23 PM Saikat Maitra <
> saikat.maitra@gmail.com>
> > > > > wrote:
> > > > >
> > > > > > Hi Andrew
> > > > > >
> > > > > >
> > > > > > I have updated the changes.
> > > > > >
> > > > > >
> > > > > > Can you please review and share feedback.
> > > > > >
> > > > > >
> > > > > > Regards
> > > > > > Saikat
> > > > > >
> > > > > >
> > > > > > On Wed, Sep 19, 2018 at 8:11 PM, Saikat Maitra <
> > > >
> > > > saikat.maitra@gmail.com>
> > > > > > wrote:
> > > > > >
> > > > > > > Hi Andrew,
> > > > > > >
> > > > > > > I have updated the tests and also added java docs.
> > > > > > >
> > > > > > > Can you please review and share feedback.
> > > > > > >
> > > > > > >
> > > > > > > Regards
> > > > > > > Saikat
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > On Sun, Sep 16, 2018 at 11:53 AM, Saikat Maitra <
> > > > >
> > > > > saikat.maitra@gmail.com>
> > > > > > > wrote:
> > > > > > >
> > > > > > > > Hi Andrew,
> > > > > > > >
> > > > > > > > I have updated the tests and also added java docs.
> > > > > > > >
> > > > > > > > Please review and share feedback.
> > > > > > > >
> > > > > > > > Regards
> > > > > > > > Saikat
> > > > > > > >
> > > > > > > >
> > > > > > > > On Sat, Sep 8, 2018 at 2:09 PM, Saikat Maitra <
> > > >
> > > > saikat.maitra@gmail.com
> > > > > >
> > > > > > > > wrote:
> > > > > > > >
> > > > > > > > > Hi Andrew, Alexey
> > > > > > > > >
> > > > > > > > > I have incorporated the review changes.
> > > > > > > > >
> > > > > > > > > I have also refactored the CacheEventSerializer
class and
> moved it
> > > >
> > > > to
> > > > > > > > > test folder because it is used only in the
> > > > >
> > > > > FlinkIgniteSourceSelfExample and
> > > > > > > > > not required for IgniteSource.
> > > > > > > > >
> > > > > > > > > Build links
> > > > >
> > > > > https://ci.ignite.apache.org/viewLog.html?buildId=1821778&;
> > > > > > > > >
> > > > > > > > > https://ci.ignite.apache.org/viewLog.html?buildId=1821774&
> ;
> > > > > > > > >
> > > > > > > > > Please review and share feedback.
> > > > > > > > >
> > > > > > > > > Regards
> > > > > > > > > Saikat
> > > > > > > > >
> > > > > > > > > On Tue, Sep 4, 2018 at 9:57 PM, Saikat Maitra
<
> > > > >
> > > > > saikat.maitra@gmail.com>
> > > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > > Hi Alexey,
> > > > > > > > > >
> > > > > > > > > > Thank you for reviewing the changes and
sharing
> feedback, I am
> > > > > > > > > > updating the PR. I will share the changes
shortly.
> > > > > > > > > >
> > > > > > > > > > Regards,
> > > > > > > > > > Saikat
> > > > > > > > > >
> > > > > > > > > > On Tue, Sep 4, 2018 at 10:59 AM, Alexey
Goncharuk <
> > > > > > > > > > alexey.goncharuk@gmail.com> wrote:
> > > > > > > > > >
> > > > > > > > > > > Hello Saikat,
> > > > > > > > > > >
> > > > > > > > > > > I see a few fellow Igniters added some
comments to
> your PR
> > > > >
> > > > > (including
> > > > > > > > > > > me).
> > > > > > > > > > > I believe the PR can be merged after
you address them.
> > > > > > > > > > >
> > > > > > > > > > > Thanks,
> > > > > > > > > > > AG
> > > > > > > > > > >
> > > > > > > > > > > пт, 31 авг. 2018 г. в 3:11,
Saikat Maitra <
> > > >
> > > > saikat.maitra@gmail.com
> > > > > > :
> > > > > > > > > > >
> > > > > > > > > > > > Thank you, Denis
> > > > > > > > > > > >
> > > > > > > > > > > > Regards,
> > > > > > > > > > > > Saikat
> > > > > > > > > > > >
> > > > > > > > > > > > On Thu, Aug 30, 2018 at 7:01 PM,
Denis Magda <
> > > >
> > > > dmagda@apache.org>
> > > > > > > > > > > wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > > Hello Saikat,
> > > > > > > > > > > > >
> > > > > > > > > > > > > Hopefully, someone from the
community will review
> the
> > > >
> > > > changes in
> > > > > > > > > > > the
> > > > > > > > > > > > > nearest time.
> > > > > > > > > > > > >
> > > > > > > > > > > > > --
> > > > > > > > > > > > > Denis
> > > > > > > > > > > > >
> > > > > > > > > > > > > On Thu, Aug 30, 2018 at 4:37
PM Saikat Maitra <
> > > > > > > > > > >
> > > > > > > > > > > saikat.maitra@gmail.com>
> > > > > > > > > > > > > wrote:
> > > > > > > > > > > > >
> > > > > > > > > > > > > > Hello,
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > The changes for IGNITE-3303
for IgniteSource is
> complete.
> > > >
> > > > This
> > > > > > > > > > > will
> > > > > > > > > > > > help
> > > > > > > > > > > > > is
> > > > > > > > > > > > > > streaming data from
Ignite cluster and process,
> filter,
> > > > > > > > > > >
> > > > > > > > > > > transform and
> > > > > > > > > > > > > > publish it back to Ignite
using IgniteSink or in
> any other
> > > > >
> > > > > data
> > > > > > > > > > > sink.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > I was hoping if the
changes can be approved I
> can go ahead
> > > > > > > > > > >
> > > > > > > > > > > merge the
> > > > > > > > > > > > > > changes.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Regards,
> > > > > > > > > > > > > > Saikat
> > > > > > > > > > > > > >
> > > > > > > > > > > > > >
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > On Tue, Aug 28, 2018
at 12:56 AM, Saikat Maitra <
> > > > > > > > > > > >
> > > > > > > > > > > > saikat.maitra@gmail.com
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > wrote:
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Hi Andrew,
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > As discussed I
have incorporated the changes.
> Please
> > > >
> > > > review
> > > > > > > > > > > and let
> > > > > > > > > > > > me
> > > > > > > > > > > > > > > know if any changes
required.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Regards,
> > > > > > > > > > > > > > > Saikat
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > On Mon, Aug 27,
2018 at 1:45 AM, Saikat Maitra
> <
> > > > > > > > > > > > >
> > > > > > > > > > > > > saikat.maitra@gmail.com>
> > > > > > > > > > > > > > > wrote:
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > Hi,
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > I have updated
the PR with additional tests.
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > Please review
and share feedback.
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > This PR is
related to IgniteSink but allows
> to stream
> > > >
> > > > data
> > > > > > > > > > > from
> > > > > > > > > > > > > Ignite.
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > PR
> https://github.com/apache/ignite/pull/870/files
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > Review
> > > > > > > > > > >
> > > > > > > > > > >
> https://reviews.ignite.apache.org/ignite/review/IGNT-CR-135
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > Regards,
> > > > > > > > > > > > > > > > Saikat
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message