Sorry my mistake. I see your point. On Tue, Sep 8, 2015 at 11:18 PM, Chandni Singh <[email protected]> wrote:
> Didn't quite understand how the backward compatibility will be broken. > If AB is operator attribute, then dt.operator.*.attr.AB is used. > Now we added AB as a port attribute so it can be declared as > dt.operator.*.port.*.attr.AB. How does it break backward compatibility? > > I mentioned dt.attr.*.SPIN_MILLIS is valid while dt.attr.SPIN_MILLIS is > not because SPIN_MILLIS is not a dag attribute. > - Chandni > > On Tue, Sep 8, 2015 at 11:01 PM, Timothy Farkas <[email protected]> > wrote: > >> Hi Chandni, >> >> Doing that would introduce backwards compatibility issues in the future. >> >> For example let's say we implement your change and we have an attribute AB >> which exists in OperatorContext today. If a developer wanted to add AB to >> PortContext in a future version, that would be a backwards incompatible >> change because it would break all the apps configs which defined >> dt.attr.AB >> (which was an acceptable way to define AB before because AB was not >> ambiguous before). >> >> A real world example of this issue would be unifier attributes >> https://malhar.atlassian.net/browse/APEX-42. Unifier attributes have all >> the same attributes as Operators except their parents in a configuration >> are a port ex. >> dt.application.*.operator.*.outputport.*.unifier.*.attr.SPIN_MILLIS. If we >> forced a different syntax for ambiguous attributes, it would be impossible >> to add something like unifier attributes in a backwards compatiblechange, >> because it would make previously unambiguous attribute declaration >> ambiguous and would cause configuration files which worked without errors >> before to start throwing errors. >> >> Tim >> >> On Tue, Sep 8, 2015 at 9:26 PM, Chandni Singh <[email protected]> >> wrote: >> >> > Hi Tim, >> > >> > With respect to syntax there isn't any difference between >> > dt.attr.CHECKPOINT_WINDOW_COUNT >> > and dt.attr.SPIN_MILLIS. >> > However behavior differs. >> > Since CHECKPOINT_WINDOW_COUNT is also a DAG attr it is not treated as >> > ambiguous while the later is. >> > >> > Seems like an application developer who sets these attributes will have >> to >> > refer the attributes in order to avoid any errors. >> > >> > Can we avoid this confusion by forcing difference in syntax? For eg. >> > dt.attr.*.SPIN_MILLIS is valid while dt.attr.SPIN_MILLIS is not? >> > >> > Chandni >> > >> > On Tue, Sep 8, 2015 at 9:56 PM, Timothy Farkas <[email protected]> >> > wrote: >> > >> > > Hi Thomas, >> > > >> > > If an ambiguous attribute like CHECKPOINT_WINDOW_COUNT is specified >> on an >> > > element which owns the attribute then that is not an ambiguous usage. >> > > Essentially nothing changes for CHECKPOINT_WINDOW_COUNT. For example: >> > > >> > > dt.attr.CHECKPOINT_WINDOW_COUNT should only set the DAG attribute, >> just >> > as >> > > it does now. >> > > dt.attr.operator.*.attr.CHECKPOINT_WINDOW_COUNT should only set the >> > > operator attribute as it does now. >> > > >> > > Another type of ambiguous attribute is SPIN_MILLIS which is defined >> for >> > > both ports and operators. There are three usage scenarios for this: >> > > >> > > 1. dt.attr.SPIN_MILLIS or dt.attr.application.*.SPIN_MILLIS >> > > This will set the same SPIN_MILLIS value for all operators and ports >> > > >> > > 2. dt.attr.operator.*.SPIN_MILLIS >> > > This will set the SPIN_MILLIS value for only operators >> > > >> > > 3. dt.attr.port.*.SPIN_MILLIS >> > > This will set the SPIN_MILLIS value for only ports >> > > >> > > It could also be possible to use the fully qualified attribute name >> of an >> > > attribute, which is the attribute name prefixed by the class name of >> the >> > > Context >> > > >> > > 4. dt.attr.com.datatorrent.api.Context.PortContext.SPIN_MILLIS >> > > This will set the attribute for ports only >> > > >> > > 5. dt.attr.com.datatorrent.api.Context.OperatorContext.SPIN_MILLIS >> > > This will set the attribute for operators only >> > > >> > > 6. >> > > >> > > >> > >> dt.attr.operator.*.attr.com.datatorrent.api.Context.PortrContext.SPIN_MILLIS >> > > This will set the attribute for ports only >> > > >> > > This changes are backwards compatible. Also two things are done to >> make >> > it >> > > easy to maintain backwards compatibility moving forward: >> > > >> > > - If a value is defined for an attribute ambiguously like >> > > dt.attr.SPIN_MILLIS . Then the same value is set on all contexts. This >> > > behavior will allow a developer to maintain backwards compatibility if >> > they >> > > add an existing attribute name to a new context. >> > > >> > > - Attributes with the same name must have the same type, if they don't >> > the >> > > unit tests fail. This prevents developers from accidentally making a >> > > backwards incompatible change. >> > > >> > > Thanks, >> > > Tim >> > > >> > > On Tue, Sep 8, 2015 at 5:40 PM, Thomas Weise <[email protected]> >> > > wrote: >> > > >> > > > How would you handle the case where two attributes with the same >> name >> > > exist >> > > > in the hierarchy, for example CHECKPOINT_WINDOW_COUNT (DAG and >> > > > OperatorContext)? >> > > > >> > > > Is the proposed change backward compatible? >> > > > >> > > > >> > > > On Tue, Sep 8, 2015 at 5:44 PM, Timothy Farkas <[email protected] >> > >> > > > wrote: >> > > > >> > > > > Hello all, >> > > > > >> > > > > This JIRA https://malhar.atlassian.net/browse/APEX-28 is >> proposing a >> > > > > change >> > > > > to the way attributes are declared in properties files. The >> proposal >> > > will >> > > > > allow a short form declaration of properties, and supports the >> > > following >> > > > > features: >> > > > > >> > > > > 1 - Root attribute declaration >> > > > > >> > > > > dt.attr.MEMORY_MB is now a valid property and will set the >> MEMORY_MB >> > > > > attribute for all operators >> > > > > >> > > > > 2 - Ambiguous property handling >> > > > > >> > > > > dt.attr.AUTO_RECORD will set the AUTO_RECORD attribute on both >> > > operators >> > > > > and ports >> > > > > >> > > > > 3 - Fully qualified attribute names >> > > > > >> > > > > Attribute names can be prefix with the name of their Context class >> > > > > >> > > > > dt.attr.com.datatorrent.api.Context.PortContext.AUTO_RECORD will >> set >> > > the >> > > > > AUTO_RECORD attribute for all ports >> > > > > >> > > > > 4 - Child attributes can be specified on parent elements >> > > > > >> > > > > dt.attr.application.*.attr.MEMORY_MB will set the MEMORY_MB >> attribute >> > > on >> > > > > all operators >> > > > > >> > > > > 5 - Existing methods for setting properties are supported. >> > > > > >> > > > > An implementation of this is pending review here: >> > > > > https://github.com/apache/incubator-apex-core/pull/11 >> > > > > >> > > > > Please provide your feedback. >> > > > > >> > > > > Thanks, >> > > > > Tim >> > > > > >> > > > >> > > >> > >> > >
