Hi Thomas, I have updated existing PR with new changes.
Regards, -Tushar. On Thu, Apr 28, 2016 at 7:08 PM, Thomas Weise <[email protected]> wrote: > Tushar, > > You can replace the existing PR. > > Thanks > > -- > sent from mobile > On Apr 27, 2016 11:57 PM, "Tushar Gosavi" <[email protected]> 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 < > [email protected]> > > wrote: > > > > > Tushar, > > > > > > Can you create an alternate pull request with this change. > > > > > > Thanks > > > > > > > On Apr 26, 2016, at 1:09 PM, Tushar Gosavi <[email protected]> > > > 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 < > [email protected]> > > > > 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 < > [email protected] > > > > > > >> 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 < > > [email protected] > > > > > > > >>> 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. > > > >> > > > >> > > > > > >
