apex-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Thomas Weise <tho...@datatorrent.com>
Subject Re: APEXMALHAR-2084 - Getters and setters for Kafka are not consistent for topics and clusters
Date Fri, 13 May 2016 05:11:00 GMT
I would suggest to consider usability over operator developer convenience.
With String[] as parameter, BeanUtils will automatically convert a comma
separated string into an array. That does not work with List<String>.

Someone could also explore the use of additional BeanUtils converters for
common types that we would like to offer better configuration is support
for. This same discussion came up a while ago with time duration/unit
properties.

Thomas

On Thu, May 12, 2016 at 9:17 PM, Bhupesh Chawda <bhupesh@datatorrent.com>
wrote:

> If this List is large enough, the user might have to create a large number
> of properties, one for each element.
> In this case however, it should be small enough and I think we can go ahead
> with a List.
>
> ~Bhupesh
>
> On Thu, May 12, 2016 at 8:36 PM, Yogi Devendra <yogidevendra@apache.org>
> wrote:
>
> > IMO from engineering perspective, List<String> is more elegant than comma
> > separated strings and parsing it later.
> >
> > From user experience perspective, it is important to have consistent way
> > across all operators. Some operators having comma separated string others
> > having List<String> is confusing.
> >
> > Since, this discussion is more generic, not specific to a particular
> > operator we should discuss this as a separate email thread. Some
> guideline
> > document describing best practices would be even better.
> >
> >
> > ~ Yogi
> >
> > On 13 May 2016 at 06:36, Munagala Ramanath <ram@datatorrent.com> wrote:
> >
> > > Philosophical musing: In software, functionality trumps (pun *not*
> > > intended) elegance.
> > >
> > > Ram
> > >
> > > On Thu, May 12, 2016 at 6:02 PM, Bhupesh Chawda <
> bhupesh@datatorrent.com
> > >
> > > wrote:
> > >
> > > > I thought that this might not be considered clean w.r.t. the user
> > > > experience while configuring the operator.
> > > > However, if it is acceptable then I'll go ahead with this.
> > > >
> > > > Thanks for pointing it out!
> > > >
> > > > ~Bhupesh
> > > >
> > > > On Thu, May 12, 2016 at 5:52 PM, Thomas Weise <
> thomas@datatorrent.com>
> > > > wrote:
> > > >
> > > > > Thanks! Looks a bit cumbersome though...
> > > > >
> > > > >
> > > > > On Thu, May 12, 2016 at 5:47 PM, Munagala Ramanath <
> > > ram@datatorrent.com>
> > > > > wrote:
> > > > >
> > > > > > An example of doing *exactly* that is shown here:
> > > > > > http://docs.datatorrent.com/application_packages/
> > > > > > in the section entitled "Operator properties"
> > > > > >
> > > > > > Ram
> > > > > >
> > > > > > On Thu, May 12, 2016 at 5:44 PM, Bhupesh Chawda <
> > > > bhupesh@datatorrent.com
> > > > > >
> > > > > > wrote:
> > > > > >
> > > > > > > AFAIK, there is no clean way to set a List property from
an XML
> > > file.
> > > > > > >
> > > > > > > ~Bhupesh
> > > > > > >
> > > > > > > On Thu, May 12, 2016 at 5:41 PM, Munagala Ramanath <
> > > > > ram@datatorrent.com>
> > > > > > > wrote:
> > > > > > >
> > > > > > > > What was the concern about "configuring the List elements
> from
> > > the
> > > > > XML
> > > > > > > > property file"
> > > > > > > > and what issue might it create ?
> > > > > > > >
> > > > > > > > Ram
> > > > > > > >
> > > > > > > > On Thu, May 12, 2016 at 5:38 PM, Bhupesh Chawda <
> > > > > > bhupesh@datatorrent.com
> > > > > > > >
> > > > > > > > wrote:
> > > > > > > >
> > > > > > > > > Hi All,
> > > > > > > > >
> > > > > > > > > I'll reopen the pull request again after addressing
review
> > > > > comments.
> > > > > > > > >
> > > > > > > > >    - There was a concern from Thomas regarding
configuring
> > the
> > > > List
> > > > > > > > >    elements from the XML property file. I think
this might
> > > create
> > > > > an
> > > > > > > > > issue. I
> > > > > > > > >    propose that in that case we create String
getters and
> > > setters
> > > > > > > instead
> > > > > > > > > of
> > > > > > > > >    list. The signatures will stay the same but
we'll change
> > the
> > > > > type
> > > > > > of
> > > > > > > > the
> > > > > > > > >    fields to String to make them consistent:
> > > > > > > > >       - String topics
> > > > > > > > >       - public String getTopics()
> > > > > > > > >       - public void setTopics(String topics)
> > > > > > > > >    - I'll rename the branch name as suggested
by Gaurav on
> > the
> > > > pull
> > > > > > > > > request.
> > > > > > > > >
> > > > > > > > > Thanks.
> > > > > > > > >
> > > > > > > > > ~Bhupesh
> > > > > > > > >
> > > > > > > > > On Wed, May 11, 2016 at 8:43 AM, Munagala Ramanath
<
> > > > > > > ram@datatorrent.com>
> > > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > > Yes, making the accessor types match the
field will avoid
> > > > > > confusion,
> > > > > > > so
> > > > > > > > > +1.
> > > > > > > > > >
> > > > > > > > > > We can add accessors like getTopicItem()
and
> setTopicItem()
> > > for
> > > > > the
> > > > > > > > > > individual
> > > > > > > > > > elements if necessary.
> > > > > > > > > >
> > > > > > > > > > Ram
> > > > > > > > > >
> > > > > > > > > > On Wed, May 11, 2016 at 12:08 AM, Bhupesh
Chawda <
> > > > > > > > > bhupesh@datatorrent.com>
> > > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > > Hi All,
> > > > > > > > > > >
> > > > > > > > > > > The fields in Kafka Input Operator
in Malhar kafka
> > project
> > > > are
> > > > > of
> > > > > > > > type
> > > > > > > > > > > String array. However the getters and
setters are not
> > > > > consistent
> > > > > > > and
> > > > > > > > > > have a
> > > > > > > > > > > return type and parameters of String
type respectively.
> > > > > > > > > > > This may cause issues in down stream
systems and hence
> > must
> > > > be
> > > > > > > > fixed. I
> > > > > > > > > > > have created a JIRA to track this -
> > > > > > > > > > > https://issues.apache.org/jira/browse/APEXMALHAR-2084
> > > > > > > > > > >
> > > > > > > > > > > I am proposing that the types of these
fields be made
> > > > > > List<String>
> > > > > > > > > > instead
> > > > > > > > > > > of String[ ]. Additionally also make
the getters and
> > > setters
> > > > > > accept
> > > > > > > > > > > parameters and return data of the same
type.
> > > > > > > > > > >
> > > > > > > > > > > Please let me know your thoughts on
the same.
> > > > > > > > > > >
> > > > > > > > > > > Thanks
> > > > > > > > > > > ~Bhupesh
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>

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