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