Skip to content
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
Merged

Conversation

rmuir
Copy link
Contributor

@rmuir 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.


if (site) {
if (!Files.exists(dir.resolve("_site"))) {
throw new IllegalArgumentException("Plugin [" + name + "] is a site plugin but has no _site");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe add directory or dir at the end of the string

@jaymode
Copy link
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
Adapt pluginmanager to the new world
@rmuir rmuir merged commit b0d8d5e into elastic:master Jul 23, 2015
@clintongormley clintongormley added >breaking :Core/Infra/Plugins Plugin API and infrastructure labels Jul 23, 2015
@clintongormley
Copy link

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
Copy link

@synhershko
Copy link
Contributor

Ack, thanks Clinton

@jprante
Copy link
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
Copy link

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
Copy link
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
Copy link
Contributor

👍

@YannBrrd
Copy link

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
Copy link

duydo commented Aug 27, 2015

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

@YannBrrd
Copy link

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

@jprante
Copy link
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
Copy link

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
Copy link

@sscarduzio
Copy link
Contributor

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

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

@YannBrrd
Copy link

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
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants