New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adapt pluginmanager to the new world #12408

Merged
merged 5 commits into from Jul 23, 2015

Conversation

Projects
None yet
9 participants
@rmuir
Contributor

rmuir commented Jul 23, 2015

Now that plugins have some structure / e.g. required descriptor file, make pluginmanager strict and prevent mistakes from screwing up the installation.

Today it is still lenient and will basically accept any .zip or .jar and who knows what will happen. There is no sense in leniency, PluginService is going to reject any bullshit on startup.

  • Read/validate descriptor file, remove heuristics around _site or not
  • Look for descriptor file to find the plugin "root" in a simpler/safer way, when plugin-a.zip expands to plugin-a/ and we have to deal with that.
  • Proper jar hell check for non-isolated plugins: we check the candidate against any other non-isolated plugins already installed.
  • Fix tests to not use embedded zip files, instead create them on the fly for testing.
@jaymode

This comment has been minimized.

Show comment
Hide comment
@jaymode

jaymode Jul 23, 2015

Member

left a few minor comments about exception messages. other than that LGTM

Member

jaymode commented Jul 23, 2015

left a few minor comments about exception messages. other than that LGTM

@rmuir rmuir removed the review label Jul 23, 2015

rmuir added a commit that referenced this pull request Jul 23, 2015

Merge pull request #12408 from rmuir/adapt_pluginmanager
Adapt pluginmanager to the new world

@rmuir rmuir merged commit b0d8d5e into elastic:master Jul 23, 2015

1 check passed

CLA Commit author is a member of Elasticsearch
Details
@clintongormley

This comment has been minimized.

Show comment
Hide comment
@clintongormley

clintongormley Jul 24, 2015

Member

Contacting community plugin authors:

@sscarduzio, @lukas-vlcek, @mobz, @jprante, @royrusso, @andrewvc, @lmenezes, @karmi, @grantr, @shikhar, @grmblfrz, @yakaz, @synhershko, @medcl, @chytreg, @imotov, @duydo, @carrot2, @wikimedia, @kzwang, @YannBrrd, @NLPchina, @codelibs

Hi all

Just to let you know about a breaking change coming to plugins in 2.0. Each plugin will require a top-level plugin-descriptor.properties which declares metadata about the plugin. The details can be found here:

https://github.com/elastic/elasticsearch/blob/master/dev-tools/src/main/resources/plugin-metadata/plugin-descriptor.properties

Also, site plugins will need to have a _site directory. We no longer move anything that isn't a jar into an auto-created _site dir.

Member

clintongormley commented Jul 24, 2015

Contacting community plugin authors:

@sscarduzio, @lukas-vlcek, @mobz, @jprante, @royrusso, @andrewvc, @lmenezes, @karmi, @grantr, @shikhar, @grmblfrz, @yakaz, @synhershko, @medcl, @chytreg, @imotov, @duydo, @carrot2, @wikimedia, @kzwang, @YannBrrd, @NLPchina, @codelibs

Hi all

Just to let you know about a breaking change coming to plugins in 2.0. Each plugin will require a top-level plugin-descriptor.properties which declares metadata about the plugin. The details can be found here:

https://github.com/elastic/elasticsearch/blob/master/dev-tools/src/main/resources/plugin-metadata/plugin-descriptor.properties

Also, site plugins will need to have a _site directory. We no longer move anything that isn't a jar into an auto-created _site dir.

@clintongormley

This comment has been minimized.

Show comment
Hide comment
@synhershko

This comment has been minimized.

Show comment
Hide comment
@synhershko

synhershko Jul 24, 2015

Contributor

Ack, thanks Clinton

Contributor

synhershko commented Jul 24, 2015

Ack, thanks Clinton

@jprante

This comment has been minimized.

Show comment
Hide comment
@jprante

jprante Jul 24, 2015

Contributor

I saw this yesterday :) https://twitter.com/xbib/status/624236671551283201

Just a side note, for testing custom jvm plugins, it is also important to add the plugin class name to the key plugin.types to the node settings to get it loaded, because auto-loading from classpath has been disabled. Like this:

settingsBuilder()
                .put("cluster.name", cluster)
                .put("path.home", getHome())
                .put("plugin.types", MyCustomPlugin.class.getName())
                .build();

Will a blog post follow to instruct the new plugin authoring?

Contributor

jprante commented Jul 24, 2015

I saw this yesterday :) https://twitter.com/xbib/status/624236671551283201

Just a side note, for testing custom jvm plugins, it is also important to add the plugin class name to the key plugin.types to the node settings to get it loaded, because auto-loading from classpath has been disabled. Like this:

settingsBuilder()
                .put("cluster.name", cluster)
                .put("path.home", getHome())
                .put("plugin.types", MyCustomPlugin.class.getName())
                .build();

Will a blog post follow to instruct the new plugin authoring?

@clintongormley

This comment has been minimized.

Show comment
Hide comment
@clintongormley

clintongormley Jul 24, 2015

Member

thanks @jprante - i'm in the process of redoing the plugin docs for 2.0 (see #12040) and I'll update this PR to include documentation.

(btw, why are none of your plugins listed? :) )

Member

clintongormley commented Jul 24, 2015

thanks @jprante - i'm in the process of redoing the plugin docs for 2.0 (see #12040) and I'll update this PR to include documentation.

(btw, why are none of your plugins listed? :) )

@jprante

This comment has been minimized.

Show comment
Hide comment
@jprante

jprante Jul 24, 2015

Contributor

@clintongormley I'm not sure, maybe they are not important, maybe I'm too lazy to work through the stuff and submit pull requests. I can't even properly document my plugins for myself or write blog posts :( I know it's really a shame.

Contributor

jprante commented Jul 24, 2015

@clintongormley I'm not sure, maybe they are not important, maybe I'm too lazy to work through the stuff and submit pull requests. I can't even properly document my plugins for myself or write blog posts :( I know it's really a shame.

@lmenezes

This comment has been minimized.

Show comment
Hide comment
@lmenezes

lmenezes Jul 25, 2015

Contributor

👍

Contributor

lmenezes commented Jul 25, 2015

👍

@YannBrrd

This comment has been minimized.

Show comment
Hide comment
@YannBrrd

YannBrrd Aug 22, 2015

Ok. I'll release a new version asap. :-)

Le sam. 25 juil. 2015 à 23:03, leonardo menezes notifications@github.com
a écrit :

[image: 👍]


Reply to this email directly or view it on GitHub
#12408 (comment)
.

Cordialement,
Yann Barraud

YannBrrd commented Aug 22, 2015

Ok. I'll release a new version asap. :-)

Le sam. 25 juil. 2015 à 23:03, leonardo menezes notifications@github.com
a écrit :

[image: 👍]


Reply to this email directly or view it on GitHub
#12408 (comment)
.

Cordialement,
Yann Barraud

@duydo

This comment has been minimized.

Show comment
Hide comment
@duydo

duydo Aug 27, 2015

Well noted. Thank @clintongormley for your information :-)

duydo commented Aug 27, 2015

Well noted. Thank @clintongormley for your information :-)

@YannBrrd

This comment has been minimized.

Show comment
Hide comment
@YannBrrd

YannBrrd Sep 13, 2015

@clintongormley @rmuir @imotov where are the Cache, CacheBuilder & ElasticsearchIllegalArgumentException gone ?
How do I manage cached configuration now ?

YannBrrd commented Sep 13, 2015

@clintongormley @rmuir @imotov where are the Cache, CacheBuilder & ElasticsearchIllegalArgumentException gone ?
How do I manage cached configuration now ?

@jprante

This comment has been minimized.

Show comment
Hide comment
@jprante

jprante Sep 13, 2015

Contributor

@YannBrrd Cache/CacheBuilder classes are part of Guava, and you can use the dependency package of Guava, com.google.common.cache. Instead of ElasticsearchIllegalArgumentException you can either use java.lang.IllegalArgumentException or use the checks in com.google.common.base.Preconditions

Contributor

jprante commented Sep 13, 2015

@YannBrrd Cache/CacheBuilder classes are part of Guava, and you can use the dependency package of Guava, com.google.common.cache. Instead of ElasticsearchIllegalArgumentException you can either use java.lang.IllegalArgumentException or use the checks in com.google.common.base.Preconditions

@YannBrrd

This comment has been minimized.

Show comment
Hide comment
@YannBrrd

YannBrrd Sep 13, 2015

Nice thanks !

Le dim. 13 sept. 2015 12:34, Jörg Prante notifications@github.com a
écrit :

@YannBrrd https://github.com/YannBrrd Cache/CacheBuilder classes are
part of Guava, and you can use the dependency package of Guava,
com.google.common.cache. Instead of ElasticsearchIllegalArgumentException
you can either use java.lang.IllegalArgumentException or use the checks
in com.google.common.base.Preconditions


Reply to this email directly or view it on GitHub
#12408 (comment)
.

Cordialement,
Yann Barraud

YannBrrd commented Sep 13, 2015

Nice thanks !

Le dim. 13 sept. 2015 12:34, Jörg Prante notifications@github.com a
écrit :

@YannBrrd https://github.com/YannBrrd Cache/CacheBuilder classes are
part of Guava, and you can use the dependency package of Guava,
com.google.common.cache. Instead of ElasticsearchIllegalArgumentException
you can either use java.lang.IllegalArgumentException or use the checks
in com.google.common.base.Preconditions


Reply to this email directly or view it on GitHub
#12408 (comment)
.

Cordialement,
Yann Barraud

@YannBrrd

This comment has been minimized.

Show comment
Hide comment
@sscarduzio

This comment has been minimized.

Show comment
Hide comment
@sscarduzio

sscarduzio Nov 10, 2015

Contributor

Hi guys, Readonly REST plugin is updated as well. 👍

https://github.com/sscarduzio/elasticsearch-readonlyrest-plugin

Contributor

sscarduzio commented Nov 10, 2015

Hi guys, Readonly REST plugin is updated as well. 👍

https://github.com/sscarduzio/elasticsearch-readonlyrest-plugin

@YannBrrd

This comment has been minimized.

Show comment
Hide comment
@YannBrrd

YannBrrd Dec 4, 2015

@jprante how do I fix the jar hell after adding guava as a dependency ? Integrations tests failing... /o\

Edit : My bad, was fixed as per ES issue

YannBrrd commented Dec 4, 2015

@jprante how do I fix the jar hell after adding guava as a dependency ? Integrations tests failing... /o\

Edit : My bad, was fixed as per ES issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment