Ignite Plugin public API

[Artiom Shutak <as...@gridgain.com>]

Hi,

I hope it's not too late to change public API of the Ignite Plugin feature.

I have next suggestions:
1. PluginConfiguration interface have only one method

Class<? extends PluginProvider> providerClass();

and we have processing code, which try to instantiate PliginProvider with 3
types of constructors (in this order): constructor with PluginContext as
parameter, constructor with PluginConfiguration as parameter and default
constructor.
It seems to me that there is no a reason for user to don't apply already
created PluginContext, which have all needed information. I suggest another
method

PluginProvider createProvider(PluginContext ctx);

I additional, I want to note that if user want to create own plugin then he
has to extend PluginConfiguration (there is no way to implement just
PluginProvider without PluginConfiguration).

2. PluginContext suggestions

- "configuration" method (return PluginConfiguration) rename to
"pluginConfiguration";
- "grid" method (return Ignite) rename to "ignite"
- to delete next methods, because all of them can be gotten from
Ignite: igniteConfiguration, nodes, localNode, log.


-- Artem --

Re: Ignite Plugin public API

[Yakov Zhdanov <yz...@apache.org>]

Hi!

Given none has yet started implementing plugins for Ignite, I think we can
do it.

I agree with both points.

1. Why we force user to have constructors we want? Will not it be simpler
just return instantiated and configured provider? Jcache uses similar
approach with its factories.
2. We can remove 4 methods from context, since everything is available via
Ignite - Ignite.log(); Ignite.cluster().localNode();
Ignite.configuration(); etc

Thoughts?

--Yakov

2015-04-03 17:52 GMT+03:00 Artiom Shutak <as...@gridgain.com>:

> Hi,
>
> I hope it's not too late to change public API of the Ignite Plugin feature.
>
> I have next suggestions:
> 1. PluginConfiguration interface have only one method
>
> Class<? extends PluginProvider> providerClass();
>
> and we have processing code, which try to instantiate PliginProvider with 3
> types of constructors (in this order): constructor with PluginContext as
> parameter, constructor with PluginConfiguration as parameter and default
> constructor.
> It seems to me that there is no a reason for user to don't apply already
> created PluginContext, which have all needed information. I suggest another
> method
>
> PluginProvider createProvider(PluginContext ctx);
>
> I additional, I want to note that if user want to create own plugin then he
> has to extend PluginConfiguration (there is no way to implement just
> PluginProvider without PluginConfiguration).
>
> 2. PluginContext suggestions
>
> - "configuration" method (return PluginConfiguration) rename to
> "pluginConfiguration";
> - "grid" method (return Ignite) rename to "ignite"
> - to delete next methods, because all of them can be gotten from
> Ignite: igniteConfiguration, nodes, localNode, log.
>
>
> -- Artem --
>

Re: Ignite Plugin public API

[Vladimir Ozerov <vo...@gridgain.com>]

Another important thing we should think about is how to provide flexible
way to choose only several features from plugin. That is, what if I want to
use DR from plugin A, but security from plugin B? Currently this is not
addressed anyhow - the first plugin to provide particular processor
implementation wins.
We can have a kind of map from "feature" to plugin, i.e. I can set
explicitly that DR is to be taken from plugin B.

On Fri, Apr 3, 2015 at 8:18 PM, Vladimir Ozerov <vo...@gridgain.com>
wrote:

> -1 for automatic plugin detection. My concerns:
> a) Different plugins might provide overlapping features and it is
> necessary to decide which service will be provided by which provider.
> b) Plugins participate in node lifecycle and relative start/stop order
> might be important as well. E.g. when there are two plugins from the same
> vendor and there is a requirement that one plugin must be started before
> another.
> c) Every plugin most likely will have configuration, so something
> "explicit" will be required anyway.
> d) Last, but not least - this approach is prone to "jar hell" issues
> typical for app servers.
>
>
> On Fri, Apr 3, 2015 at 7:11 PM, Sergey Evdokimov <se...@gridgain.com>
> wrote:
>
>> Java provides standard way to load application extensions.
>> See java.util.ServiceLoader. We can use it to avoid reinventing the wheel.
>>
>> Look how java cache system detects CachingProvider classes: the jar that
>> provides cache implementation registers his CachingProvider class name
>> in /META-INF/services/javax.cache.spi.CachingProvider file. Then cache
>> provider will be found automatically using java.util.ServiceLoader.
>>
>> Ignite may detect plugin providers same way. User will not need to
>> describe
>> each plugin in configuration, adding plugin jar to classpath will be
>> enough. But this approach requires total refactoring of Ignite plugin
>> system.
>>
>> On Fri, Apr 3, 2015 at 5:52 PM, Artiom Shutak <as...@gridgain.com>
>> wrote:
>>
>> > Hi,
>> >
>> > I hope it's not too late to change public API of the Ignite Plugin
>> feature.
>> >
>> > I have next suggestions:
>> > 1. PluginConfiguration interface have only one method
>> >
>> > Class<? extends PluginProvider> providerClass();
>> >
>> > and we have processing code, which try to instantiate PliginProvider
>> with 3
>> > types of constructors (in this order): constructor with PluginContext as
>> > parameter, constructor with PluginConfiguration as parameter and default
>> > constructor.
>> > It seems to me that there is no a reason for user to don't apply already
>> > created PluginContext, which have all needed information. I suggest
>> another
>> > method
>> >
>> > PluginProvider createProvider(PluginContext ctx);
>> >
>> > I additional, I want to note that if user want to create own plugin
>> then he
>> > has to extend PluginConfiguration (there is no way to implement just
>> > PluginProvider without PluginConfiguration).
>> >
>> > 2. PluginContext suggestions
>> >
>> > - "configuration" method (return PluginConfiguration) rename to
>> > "pluginConfiguration";
>> > - "grid" method (return Ignite) rename to "ignite"
>> > - to delete next methods, because all of them can be gotten from
>> > Ignite: igniteConfiguration, nodes, localNode, log.
>> >
>> >
>> > -- Artem --
>> >
>>
>
>

Re: Ignite Plugin public API

[Konstantin Boudnik <co...@apache.org>]

On Fri, Apr 03, 2015 at 08:18PM, Vladimir Ozerov wrote:
> -1 for automatic plugin detection. My concerns:
> a) Different plugins might provide overlapping features and it is necessary
> to decide which service will be provided by which provider.

While the rest of your points are pretty valid, IMO, I'd say that guessing a
user intention might not work as well as anticipated. If a user messed up and
have two plugins of the conflicting nature setup - let him deal with the
consequences. 

"Unix gives you enough rope to hand yourself and then a couple more feet to make sure"

Cos

> b) Plugins participate in node lifecycle and relative start/stop order
> might be important as well. E.g. when there are two plugins from the same
> vendor and there is a requirement that one plugin must be started before
> another.
> c) Every plugin most likely will have configuration, so something
> "explicit" will be required anyway.
> d) Last, but not least - this approach is prone to "jar hell" issues
> typical for app servers.
> 
> 
> On Fri, Apr 3, 2015 at 7:11 PM, Sergey Evdokimov <se...@gridgain.com>
> wrote:
> 
> > Java provides standard way to load application extensions.
> > See java.util.ServiceLoader. We can use it to avoid reinventing the wheel.
> >
> > Look how java cache system detects CachingProvider classes: the jar that
> > provides cache implementation registers his CachingProvider class name
> > in /META-INF/services/javax.cache.spi.CachingProvider file. Then cache
> > provider will be found automatically using java.util.ServiceLoader.
> >
> > Ignite may detect plugin providers same way. User will not need to describe
> > each plugin in configuration, adding plugin jar to classpath will be
> > enough. But this approach requires total refactoring of Ignite plugin
> > system.
> >
> > On Fri, Apr 3, 2015 at 5:52 PM, Artiom Shutak <as...@gridgain.com>
> > wrote:
> >
> > > Hi,
> > >
> > > I hope it's not too late to change public API of the Ignite Plugin
> > feature.
> > >
> > > I have next suggestions:
> > > 1. PluginConfiguration interface have only one method
> > >
> > > Class<? extends PluginProvider> providerClass();
> > >
> > > and we have processing code, which try to instantiate PliginProvider
> > with 3
> > > types of constructors (in this order): constructor with PluginContext as
> > > parameter, constructor with PluginConfiguration as parameter and default
> > > constructor.
> > > It seems to me that there is no a reason for user to don't apply already
> > > created PluginContext, which have all needed information. I suggest
> > another
> > > method
> > >
> > > PluginProvider createProvider(PluginContext ctx);
> > >
> > > I additional, I want to note that if user want to create own plugin then
> > he
> > > has to extend PluginConfiguration (there is no way to implement just
> > > PluginProvider without PluginConfiguration).
> > >
> > > 2. PluginContext suggestions
> > >
> > > - "configuration" method (return PluginConfiguration) rename to
> > > "pluginConfiguration";
> > > - "grid" method (return Ignite) rename to "ignite"
> > > - to delete next methods, because all of them can be gotten from
> > > Ignite: igniteConfiguration, nodes, localNode, log.
> > >
> > >
> > > -- Artem --
> > >
> >

Re: Ignite Plugin public API

[Vladimir Ozerov <vo...@gridgain.com>]

-1 for automatic plugin detection. My concerns:
a) Different plugins might provide overlapping features and it is necessary
to decide which service will be provided by which provider.
b) Plugins participate in node lifecycle and relative start/stop order
might be important as well. E.g. when there are two plugins from the same
vendor and there is a requirement that one plugin must be started before
another.
c) Every plugin most likely will have configuration, so something
"explicit" will be required anyway.
d) Last, but not least - this approach is prone to "jar hell" issues
typical for app servers.


On Fri, Apr 3, 2015 at 7:11 PM, Sergey Evdokimov <se...@gridgain.com>
wrote:

> Java provides standard way to load application extensions.
> See java.util.ServiceLoader. We can use it to avoid reinventing the wheel.
>
> Look how java cache system detects CachingProvider classes: the jar that
> provides cache implementation registers his CachingProvider class name
> in /META-INF/services/javax.cache.spi.CachingProvider file. Then cache
> provider will be found automatically using java.util.ServiceLoader.
>
> Ignite may detect plugin providers same way. User will not need to describe
> each plugin in configuration, adding plugin jar to classpath will be
> enough. But this approach requires total refactoring of Ignite plugin
> system.
>
> On Fri, Apr 3, 2015 at 5:52 PM, Artiom Shutak <as...@gridgain.com>
> wrote:
>
> > Hi,
> >
> > I hope it's not too late to change public API of the Ignite Plugin
> feature.
> >
> > I have next suggestions:
> > 1. PluginConfiguration interface have only one method
> >
> > Class<? extends PluginProvider> providerClass();
> >
> > and we have processing code, which try to instantiate PliginProvider
> with 3
> > types of constructors (in this order): constructor with PluginContext as
> > parameter, constructor with PluginConfiguration as parameter and default
> > constructor.
> > It seems to me that there is no a reason for user to don't apply already
> > created PluginContext, which have all needed information. I suggest
> another
> > method
> >
> > PluginProvider createProvider(PluginContext ctx);
> >
> > I additional, I want to note that if user want to create own plugin then
> he
> > has to extend PluginConfiguration (there is no way to implement just
> > PluginProvider without PluginConfiguration).
> >
> > 2. PluginContext suggestions
> >
> > - "configuration" method (return PluginConfiguration) rename to
> > "pluginConfiguration";
> > - "grid" method (return Ignite) rename to "ignite"
> > - to delete next methods, because all of them can be gotten from
> > Ignite: igniteConfiguration, nodes, localNode, log.
> >
> >
> > -- Artem --
> >
>

Re: Ignite Plugin public API

[Sergey Evdokimov <se...@gridgain.com>]

Java provides standard way to load application extensions.
See java.util.ServiceLoader. We can use it to avoid reinventing the wheel.

Look how java cache system detects CachingProvider classes: the jar that
provides cache implementation registers his CachingProvider class name
in /META-INF/services/javax.cache.spi.CachingProvider file. Then cache
provider will be found automatically using java.util.ServiceLoader.

Ignite may detect plugin providers same way. User will not need to describe
each plugin in configuration, adding plugin jar to classpath will be
enough. But this approach requires total refactoring of Ignite plugin
system.

On Fri, Apr 3, 2015 at 5:52 PM, Artiom Shutak <as...@gridgain.com> wrote:

> Hi,
>
> I hope it's not too late to change public API of the Ignite Plugin feature.
>
> I have next suggestions:
> 1. PluginConfiguration interface have only one method
>
> Class<? extends PluginProvider> providerClass();
>
> and we have processing code, which try to instantiate PliginProvider with 3
> types of constructors (in this order): constructor with PluginContext as
> parameter, constructor with PluginConfiguration as parameter and default
> constructor.
> It seems to me that there is no a reason for user to don't apply already
> created PluginContext, which have all needed information. I suggest another
> method
>
> PluginProvider createProvider(PluginContext ctx);
>
> I additional, I want to note that if user want to create own plugin then he
> has to extend PluginConfiguration (there is no way to implement just
> PluginProvider without PluginConfiguration).
>
> 2. PluginContext suggestions
>
> - "configuration" method (return PluginConfiguration) rename to
> "pluginConfiguration";
> - "grid" method (return Ignite) rename to "ignite"
> - to delete next methods, because all of them can be gotten from
> Ignite: igniteConfiguration, nodes, localNode, log.
>
>
> -- Artem --
>