Skip to content

Plugin zip support#4975

Closed
BigFunger wants to merge 7 commits into
elastic:masterfrom
BigFunger:plugin-zip-support
Closed

Plugin zip support#4975
BigFunger wants to merge 7 commits into
elastic:masterfrom
BigFunger:plugin-zip-support

Conversation

@BigFunger
Copy link
Copy Markdown
Contributor

Fixes #4720

Adds zip support to the plugin installer.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think for modules, you want to have those declared with const

@epixa
Copy link
Copy Markdown
Contributor

epixa commented Sep 18, 2015

Two things:

  1. I cannot get the .zip extraction to work as expected, and I'm not sure if it's due to some mechanics of zip itself that I'm not familiar with or simply the way the contents are being extracted here. In any case, if I unzip the contents of a zip using OS X, I get a correct plugin directory structure. If I run the same file (test-plugin-master.zip) through this installer, the contents of the plugin get installed to installedPlugins/test-plugin/test-plugin-master/ rather than in installedPlugins/test-plugin. This happened for both a zip I created manually and the one you provided me. The installer runs fine, but warnings are thrown when starting kibana.
  2. I really think this code needs some tests. At the most, all I can do is confirm that it works as-is, but without tests we have zero confidence that this continues to work as we expect going forward.

@epixa epixa assigned BigFunger and unassigned epixa Sep 18, 2015
Comment thread src/cli/plugin/pluginDownloader.js Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You're not escaping the .'s, and I'm pretty sure that's a typo. Unless you really are trying to match filenamextarxgz

@rashidkpc rashidkpc added v4.3.0 and removed v4.2.0 labels Sep 22, 2015
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

All these lets can be changed to const

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants