Right, the static factory is very simple as you said. It's pretty
convenient for the client to use.

I am working on the ApplicationRunner SEP right now. Will send out the
discussion email once I am done.

Thanks,
Xinyu

On Thu, Mar 16, 2017 at 4:50 PM, Navina Ramesh (Apache) <nav...@apache.org>
wrote:

> > One minor thing I found is that the name of the config is camel case
> (*processor.idGenerator.class*). Seems Samza's practice is to use all
> lower
> case configs with "." delimiter. Do you think we should stick to this
> convention?
>
> I am always torn between the "convention" we have and the better way of
> doing things. But I don't have strong opinions about it. I can change it.
>
> > One more suggestion is to have a static factory method in the
> ProcessorIdGenerator (Like what we have in ApplicationRunner):
>
> I couldn't grasp these requirements from the ApplicationRunner design. It
> will be great if you can put it out in an SEP :)
>
> I can add the static factory method for it. Just to clarify, the static
> method simply class loads the ProcessorIdGenerator ? It uses reflection to
> create the instance ?
>
> Thanks!
> Navina
>
>
>
> On Thu, Mar 16, 2017 at 4:31 PM, xinyu liu <xinyuliu...@gmail.com> wrote:
>
> > The proposal looks great to me! Changing the id type to string will make
> > sure this can work with other types of cluster which doesn't support
> > integer id. The interface and config provides a pluggable way to have
> > different id generators for different use cases. One minor thing I found
> is
> > that the name of the config is camel case (*processor.idGenerator.class*
> ).
> > Seems Samza's practice is to use all lower case configs with "."
> delimiter.
> > Do you think we should stick to this convention?
> >
> > One more suggestion is to have a static factory method in
> > the ProcessorIdGenerator (Like what we have in ApplicationRunner):
> >
> > static ProcessIdGenerator fromConfig(Config config) { ... }.
> >
> > With this, It will be more convenient for the ApplicationRunner to
> > construct the generator. What do you think?
> >
> > Thanks,
> > Xinyu
> >
> > On Wed, Mar 15, 2017 at 10:59 PM, Navina Ramesh (Apache) <
> > nav...@apache.org>
> > wrote:
> >
> > > Hi everyone,
> > > I created a proposal for SAMZA-1126, which addresses the semantics of
> > > ProcessorId in Samza. For most purposes, ProcessorId is same as the
> > logical
> > > id that Samza assigns for each Yarn container. It is primarily used in
> > > JobModel as a key for the corresponding ContainerModel and also, in
> > > container-level metrics. We are expanding the applicability of
> > processorId
> > > to be beyond a fixed set of processors.
> > >
> > > Please review and comment on this SEP.
> > >
> > > For those who are not actively following the master branch, you may
> have
> > > more questions than others. Feel free to ask them here.
> > >
> > > @Xinyu: Since you are working on SAMZA-1067 and other related
> integration
> > > APIs, can you please add an SEP for SAMZA-1067 ? This will help others
> > (adn
> > > me as well) get on the same page with your design/code. Let me know if
> > > SEP-1 will work per your design for ApplicationRunner.
> > >
> > > Thanks!
> > > Navina
> > >
> >
>

Reply via email to