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

Display plugins versions #7221

Merged
merged 1 commit into from
May 31, 2016
Merged

Display plugins versions #7221

merged 1 commit into from
May 31, 2016

Conversation

ggrossetie
Copy link
Contributor

This is useful to determine if a plugin needs to be updated when using deployment automation solution (like Ansible).

 ./kibana-plugin list
marvel@2.3.2

@elasticsearch-release
Copy link

Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run; then say 'jenkins, test it'.

import { join } from 'path';

function readable(path) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

fs.existsSync is deprecated and I think it's better to check that we can read the file: https://nodejs.org/api/fs.html#fs_fs_existssync_path
Is there a better place to put this utility function ?

Copy link
Member

@jbudz jbudz May 18, 2016

Choose a reason for hiding this comment

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

in this case it shouldn't be needed, readFileSync will already throw if the file isn't available.

thoughts on something like:

try {
  var packageJSON = JSON.parse(readFileSync(packagePath, 'utf8'));
  logger.log(filename + '@' + packageJSON.version);
} catch(e) {
  logger.log(filename);
}

?

Copy link
Contributor

@epixa epixa May 18, 2016

Choose a reason for hiding this comment

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

For 5.0+, plugins are required to have package.json (the installer will fail without that file). You should be able to assume that they exist, and a thrown error would be appropriate if the file didn't exist or was not readable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

in this case it shouldn't be needed, readFileSync will already throw if the file isn't available.

True but I didn't want to put logger.log(filename) in the catch.

For 5.0+, plugins are required to have package.json (the installer will fail without that file). You should be able to assume that they exist, and a thrown error would be appropriate if the file didn't exist or was not readable.

Ok, something like: throw Error('Unable to read package.json for ' + filename) ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, that could work. Don't forget the new operator though.

@ggrossetie
Copy link
Contributor Author

@epixa I think it would be nice to have the same behavior between Elastic stack products (ie. Elasticsearch and maybe Logstash). Wdyt ?

logger.log(filename);
const packagePath = join(settings.pluginDir, filename, 'package.json');
if (readable(packagePath)) {
var packageJSON = JSON.parse(readFileSync(packagePath, 'utf8'));
Copy link
Contributor

Choose a reason for hiding this comment

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

Since you only need version, I'd recommend using destructuring:

const { version } = JSON.parse(readFileSync(packagePath, 'utf8'));

One way or another, it should be a const rather than var.

@jbudz
Copy link
Member

jbudz commented May 27, 2016

jenkins, test it

@jbudz
Copy link
Member

jbudz commented May 27, 2016

@Mogztter thanks for the updates! It looks like we need a little more, tests in src/cli_plugin/list__tests__/list.js are mocking plugins but not writing a package.json/version causing this to throw.

@ggrossetie
Copy link
Contributor Author

@jbudz Got it, sorry about that. I had troubles running tests locally 😣

I've updated the existing tests and added two more 😄

@epixa
Copy link
Contributor

epixa commented May 28, 2016

jenkins, test it

This is useful to determine if a plugin needs to be updated when using deployment automation solution (like Ansible)
@ggrossetie
Copy link
Contributor Author

Green bars 🎉

@jbudz
Copy link
Member

jbudz commented May 31, 2016

Code LGTM

@jbudz jbudz merged commit 00f4538 into elastic:master May 31, 2016
@jbudz
Copy link
Member

jbudz commented May 31, 2016

Thanks @Mogztter!

@ggrossetie
Copy link
Contributor Author

My pleasure!

jbudz pushed a commit that referenced this pull request Sep 27, 2023
`v88.3.0`⏩`v88.5.0`

closes #151514

---

## [`88.5.0`](https://github.com/elastic/eui/tree/v88.5.0)

- Updated `EuiCallOut` with a new `onDismiss` prop
([#7156](elastic/eui#7156))
- Added a new `renderCustomToolbar` prop to `EuiDataGrid`, which allows
custom rendering of the toolbar.
([#7190](elastic/eui#7190))
- Added a new `allowResetButton` prop to
`toolbarVisibility.showDisplaySelector` of `EuiDataGrid`, which allows
hiding the "Reset to default" button from the display settings popover.
([#7190](elastic/eui#7190))
- Added a new `additionalDisplaySettings` prop to
`toolbarVisibility.showDisplaySelector` of `EuiDataGrid`, which allows
rendering extra settings inside the display settings popover.
([#7190](elastic/eui#7190))
- Updated `EuiDataGrid`'s toolbar display settings button icon
([#7190](elastic/eui#7190))
- Updated `EuiTextTruncate` with significantly improved iteration
performance. Removed `measurementRenderAPI` prop, as `EuiTextTruncation`
now only uses more performant canvas render API
([#7210](elastic/eui#7210))
- Updated `EuiPopover` with a new configurable `repositionToCrossAxis`
prop ([#7211](elastic/eui#7211))
- Updated `EuiDatePicker` to support `compressed` input styling
([#7218](elastic/eui#7218))
- Added `gradient` and `palette` icon glyphs.
([#7220](elastic/eui#7220))

**Bug fixes**

- Fixed `EuiPopover`'s missing animations on popover close
([#7211](elastic/eui#7211))
- Fixed `EuiInputPopover` anchoring to the wrong side and missing
shadows on smaller screens
([#7211](elastic/eui#7211))
- Fixed `EuiSuperDatePicker` icon spacing on the quick select button
([#7217](elastic/eui#7217))
- Fixed a missing type in `EuiMarkdownEditor`'s default processing
plugins ([#7221](elastic/eui#7221))


## [`88.4.1`](https://github.com/elastic/eui/tree/v88.4.1)

**Bug fixes**

- Fixed missing `className`s on `EuiTextTruncate`
([#7212](elastic/eui#7212))
- Fixed `title`s on `EuiComboBox` dropdown options to always be present
([#7212](elastic/eui#7212))
- Fixed `EuiComboBox` truncation issues when search is an empty space
([#7212](elastic/eui#7212))

## [`88.4.0`](https://github.com/elastic/eui/tree/v88.4.0)

- Updated `EuiComboBox` to allow configuring text truncation behavior
via `truncationProps`. These props can be set on the entire combobox as
well as on on individual dropdown options.
([#7028](elastic/eui#7028))
- Updated `EuiInMemoryTable` with a new `searchFormat` prop (defaults to
`eql`). When setting this prop to `text`, the built-in search bar will
ignore EQL syntax and allow searching for plain strings with special
characters and symbols.
([#7175](elastic/eui#7175))

**Bug fixes**

- `EuiComboBox` now always shows the highlighted search text, even on
truncated text ([#7028](elastic/eui#7028))
- Fixed missing i18n in `EuiSearchBar`'s default placeholder and
aria-label text ([#7175](elastic/eui#7175))
- Fixed the inline compressed styles of `EuiDescriptionListTitle` to use
a taller line-height for readability
([#7185](elastic/eui#7185))
- Fixed `EuiComboBox` to correctly truncate selected items when
displayed as pills and plain text
([#7193](elastic/eui#7193))

**Accessibility**

- Added `aria-current` attribute to `EuiTablePagination`
([#7186](elastic/eui#7186))

**CSS-in-JS conversions**

- Converted `EuiDroppable` and `EuiDraggable` to Emotion; Removed
`$euiDragAndDropSpacing` Sass variables
([#7187](elastic/eui#7187))

---------

Co-authored-by: Patryk Kopycinski <contact@patrykkopycinski.com>
Co-authored-by: Jan Monschke <jan.monschke@elastic.co>
Co-authored-by: Thomas Watson <watson@elastic.co>
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.

None yet

4 participants