Skip to content

Conversation

rjernst
Copy link
Member

@rjernst rjernst commented Jul 22, 2022

The new stable plugin api will have a slightly different descriptor file
format. This commit prepares for stable plugins by adding support for
reading those new files. The basic info for a plugin is the same like
name and version info. Other stuff like classname are not necessary. The
one additional property specific to the new plugins is "modular", which
indicates whether the jars of the plugin should be loaded as named
modules (this is akin to setting the module path when running java).

The new stable plugin api will have a slightly different descriptor file
format. This commit prepares for stable plugins by adding support for
reading those new files. The basic info for a plugin is the same like
name and version info. Other stuff like classname are not necessary. The
one additional property specific to the new plugins is "modular", which
indicates whether the jars of the plugin should be loaded as named
modules (this is akin to setting the module path when running java).
@rjernst rjernst added :Core/Infra/Plugins Plugin API and infrastructure >refactoring v8.4.0 labels Jul 22, 2022
@rjernst rjernst requested a review from grcevski July 22, 2022 15:34
@elasticsearchmachine elasticsearchmachine added the Team:Core/Infra Meta label for core/infra team label Jul 22, 2022
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@elasticsearchmachine elasticsearchmachine changed the base branch from master to main July 22, 2022 23:04
Copy link
Contributor

@grcevski grcevski left a comment

Choose a reason for hiding this comment

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

LGTM! Looks great, I really like how all the tests simply run for both. It looks like there are two more tests that need to be adjusted for the new missing descriptors error message.

@rjernst rjernst added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Jul 23, 2022
Copy link
Contributor

@ChrisHegarty ChrisHegarty left a comment

Choose a reason for hiding this comment

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

LGTM

@elasticsearchmachine elasticsearchmachine merged commit 0986d8b into elastic:main Jul 26, 2022
@rjernst rjernst deleted the plugins/new_descriptor2 branch July 26, 2022 00:58
rjernst added a commit to rjernst/elasticsearch that referenced this pull request Jul 26, 2022
A forgotten piece of elastic#88731
is whether a plugin descriptor in memory came from a stable or internal
descriptor. This commit adds a flag for that. Note that this is implied
by the file that was loaded, so no new property is needed.
rjernst added a commit that referenced this pull request Jul 26, 2022
A forgotten piece of #88731
is whether a plugin descriptor in memory came from a stable or internal
descriptor. This commit adds a flag for that. Note that this is implied
by the file that was loaded, so no new property is needed.
@pgomulka pgomulka mentioned this pull request Aug 1, 2022
25 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) :Core/Infra/Plugins Plugin API and infrastructure >refactoring Team:Core/Infra Meta label for core/infra team v8.4.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants