Mailing-List: contact dev-help@apex.incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@apex.incubator.apache.org
MIME-Version: 1.0
In-Reply-To: 
 <CAKJfLDMB2r07d7yyPAjfqgZqK42griK9GzbyEjS5rMoZ1DDTzQ@mail.gmail.com>
References: 
 <CAD0KA=Pg0AT1qsvh_R7aVN6iVWv-88JO0ncq64qgVupa7sJ4pw@mail.gmail.com>
	<CAKJfLDOh0Fr=uCnhpDoozUN9-yP34VJjc7Qor4ET4Q8gpTbHTQ@mail.gmail.com>
	<CAD0KA=PNdkk_fmufBtXC2J791xZhR_48HO9Rnmib2M3bxbsrgg@mail.gmail.com>
	<CAD0KA=PF6nH7A2W8un=wkixLBEQXwvmLc8Ss5vpdL79HtfAa=g@mail.gmail.com>
	<3373380032047352570@unknownmsgid>
	<CAD0KA=NJh-g8WOLfvhWWM07nEkCcKMd45dQ5BrRpBJPCPfvg5w@mail.gmail.com>
	<CAKJfLDMB2r07d7yyPAjfqgZqK42griK9GzbyEjS5rMoZ1DDTzQ@mail.gmail.com>
Date: Thu, 28 Apr 2016 23:53:34 +0530
Message-ID: 
 <CAD0KA=Mhq4faWwi7XWcADex7E4Lt97EZQdAqfm=wEnCb=WvX7Q@mail.gmail.com>
Subject: Re: [APEXCORE-107] Adding modules in DAG through property and json
 file.
From: Tushar Gosavi <tushar@datatorrent.com>
To: dev@apex.incubator.apache.org
Content-Type: multipart/alternative; boundary=94eb2c0bacb0cadde505318f9f0d

--94eb2c0bacb0cadde505318f9f0d
Content-Type: text/plain; charset=UTF-8

Hi Thomas,

I have updated existing PR with new changes.

Regards,
-Tushar.


On Thu, Apr 28, 2016 at 7:08 PM, Thomas Weise <thomas@datatorrent.com>
wrote:

> Tushar,
>
> You can replace the existing PR.
>
> Thanks
>
> --
> sent from mobile
> On Apr 27, 2016 11:57 PM, "Tushar Gosavi" <tushar@datatorrent.com> wrote:
>
> > Hi Pramod,
> >
> > Do you want me to close the current pull request, and open a new pull
> > request with this change?
> >
> > - Tushar.
> >
> >
> > On Thu, Apr 28, 2016 at 11:46 AM, Pramod Immaneni <
> pramod@datatorrent.com>
> > wrote:
> >
> > > Tushar,
> > >
> > > Can you create an alternate pull request with this change.
> > >
> > > Thanks
> > >
> > > > On Apr 26, 2016, at 1:09 PM, Tushar Gosavi <tushar@datatorrent.com>
> > > wrote:
> > > >
> > > > Hi All,
> > > >
> > > > We could achieve this by adding a common interface between Operator
> and
> > > > Module without backward compatibility issue. I did a quick prototype
> > with
> > > > this approach and was able to run
> > > > PiDemo application from Malhar built against previous version of
> Apex.
> > > With
> > > > this changes
> > > > module writer don't have to override lifecycle methods required by
> > > Operator.
> > > >
> > > > Let me know if this approach is workable. I will update the pull
> > request
> > > > #313 with more polished
> > > > changes.
> > > >
> > > > Regards,
> > > > - Tushar.
> > > >
> > > >
> > > > On Wed, Apr 6, 2016 at 1:52 AM, Tushar Gosavi <
> tushar@datatorrent.com>
> > > > wrote:
> > > >
> > > >> Hi Thomas,
> > > >>
> > > >> I agree with you that module is a composite operator except its not
> > > >> present at the execution time. Actually I feel that there should be
> > > another
> > > >> Interface `Node` which specifies a node in the DAG, from which
> > Operator
> > > and
> > > >> Module interfaces are extended. This way we may support
> transformation
> > > at
> > > >> logicalPlan level in future. Adding this will break the
> compatibility
> > > with
> > > >> current code. Hence my suggestion was to extend Module form Operator
> > as
> > > it
> > > >> is considered as composite operator.
> > > >>
> > > >> Another motivation is avoid code duplication and work duplication to
> > > >> support modules in json format, in high level api or new future
> api's
> > > that
> > > >> will support adding module and operator to DAG.
> > > >>
> > > >> Currently we extract the port information using reflection, which
> can
> > > work
> > > >> regardless of interface hierarchy, but meta object associated with
> > Port
> > > has
> > > >> reference to OperatorMeta. Changing it would result in many changes
> in
> > > >> LogicalPlan without any common interface.
> > > >>
> > > >> Thanks,
> > > >> -Tushar.
> > > >>
> > > >>
> > > >> On Tue, Mar 29, 2016 at 8:37 PM, Thomas Weise <
> thomas@datatorrent.com
> > >
> > > >> wrote:
> > > >>
> > > >>> Tushar,
> > > >>>
> > > >>> I agree with the motivation of #2 but not with the specific changes
> > you
> > > >>> suggest. "Module" is a composite operator from a users perspective,
> > but
> > > >>> the
> > > >>> operator interface defines callbacks for the execution layer that
> are
> > > not
> > > >>> applicable to "Module".
> > > >>>
> > > >>> Also, ports are fields, they can be discovered regardless of
> > interface
> > > >>> hierarchy.
> > > >>>
> > > >>> Thomas
> > > >>>
> > > >>> On Wed, Mar 23, 2016 at 3:55 AM, Tushar Gosavi <
> > tushar@datatorrent.com
> > > >
> > > >>> wrote:
> > > >>>
> > > >>>> Hi All,
> > > >>>>
> > > >>>> I am planning provide support for adding modules into the json and
> > > >>> property
> > > >>>> file specification of DAG. We will go with the same syntax as
> > > discussed
> > > >>> in
> > > >>>> following mail thread.
> > > >>>
> > >
> >
> https://mail-archives.apache.org/mod_mbox/incubator-apex-dev/201512.mbox/%3C565D0E2C.2000805%40datatorrent.com%3E
> > > >>>>
> > > >>>>
> > > >>>> There are multiple choices for the design and I want some
> suggestion
> > > >>> from
> > > >>>> the community aboutg how to go about adding this support. To
> support
> > > >>>> json/property format, Module should support following
> functionality
> > > >>>> - Property format uses setSource, addSink methods of StreamMeta,
> > which
> > > >>> were
> > > >>>> not properly supported by Module.
> > > >>>> - Module need capability of extracting port object given names.
> > > Operator
> > > >>>> provides this functionality through Operators.describe.
> > > >>>>
> > > >>>> Approach 1)
> > > >>>> As module meta and operator meta shares common fields such as
> name,
> > > port
> > > >>>> information. We can separate this out in a common class NodeMeta
> and
> > > >>> derive
> > > >>>> OperatorMeta and ModuleMeta from it. This class will handle
> > extracting
> > > >>> port
> > > >>>> information form the object. The changes involve are
> > > >>>>    - Split OperatorMeta object
> > > >>>>    - PortMeta objects will contain NodeMeta references than
> > > >>> OperatorMeta,
> > > >>>> in some places we will have to perform unchecked cast from
> NodeMeta
> > to
> > > >>>> OperatorMeta. where code was expecting OperatorMeta from the
> > > >>>> PortMeta.
> > > >>>>    - Support setSource and addSink methods.
> > > >>>>    - Change Operators.describe to accept Module or Operator and
> > return
> > > >>>> port mapping.
> > > >>>>    - Need instanceof call at few places to check if object is
> Module
> > > or
> > > >>>> Operator, before calling specific API while working with property
> > and
> > > >>> json
> > > >>>> file.
> > > >>>>    - change signature of few method which accepts Operator to
> > Object.
> > > >>> as
> > > >>>> Module and Operator do not share a common parent but requires
> > > >>>> common processing
> > > >>>>    - Replace OperatorMeta with NodeMeta in most of the classes.
> > > >>>>
> > > >>>>
> > > >>>> Approach 2)
> > > >>>> Make Module extends Operator and make ModuleMeta extends
> > OperatorMeta.
> > > >>> The
> > > >>>> changes involved will be
> > > >>>> - Change Module interface to extend Operator
> > > >>>> - add support for setSource and addSink for Modules.
> > > >>>> - addOperator will inspect type of object and call addModule if
> > > >>> operator
> > > >>>> is a module.
> > > >>>>
> > > >>>> Disadvantage
> > > >>>> - This will break compatibility but this should not be a problem
> as
> > > >>> Module
> > > >>>> is still an evolving API.
> > > >>>> - Operator lifecyle methods will not get executed for module and
> we
> > > can
> > > >>>> document this.
> > > >>>>
> > > >>>> Advantages
> > > >>>> - This will avoid code duplication at multiple places where there
> is
> > > >>>> similarity between Module and Operator, and there is lot of
> > similarity
> > > >>>> while defining logical DAG.
> > > >>>> - This will also automatically add support for Module in api being
> > > >>>> developed for operator. For example high level api.
> > > >>>> - This will make module and operator interchangeable in
> application.
> > > >>>>
> > > >>>> I will prefer Approach 2.
> > > >>>>
> > > >>>> Regards,
> > > >>>> -Tushar.
> > > >>
> > > >>
> > >
> >
>

--94eb2c0bacb0cadde505318f9f0d--