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.

Sign up for GitHub

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

Jump to bottom

Implement recursive plugin discovery #68811

Merged
merged 18 commits into from
Jun 30, 2020
Merged

Implement recursive plugin discovery #68811

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
debfb67
implements recursive scanning in plugin discovery system
pgayvallet Jun 10, 2020
8228bcd
update optimizer to find plugins in sub-directories
pgayvallet Jun 10, 2020
0cf4f90
update renovate
pgayvallet Jun 10, 2020
5645711
update optimizer IT snapshot
pgayvallet Jun 11, 2020
1a398ca
Merge remote-tracking branch 'upstream/master' into kbn-59189-recursi…
pgayvallet Jun 11, 2020
7f68ba1
Merge remote-tracking branch 'upstream/master' into kbn-59189-recursi…
pgayvallet Jun 15, 2020
c709bed
Merge remote-tracking branch 'upstream/master' into kbn-59189-recursi…
pgayvallet Jun 16, 2020
f011c93
Merge remote-tracking branch 'upstream/master' into kbn-59189-recursi…
pgayvallet Jun 23, 2020
5c00548
refactor processPluginSearchPaths$ and add test for inaccessible mani…
pgayvallet Jun 23, 2020
3cea30e
Merge remote-tracking branch 'upstream/master' into kbn-59189-recursi…
pgayvallet Jun 24, 2020
fa308bf
add symlink test
pgayvallet Jun 24, 2020
245cc4f
add maxDepth to the optimizer
pgayvallet Jun 26, 2020
6bc7cc8
adapt mockFs definitions
pgayvallet Jun 26, 2020
f25b051
Merge remote-tracking branch 'upstream/master' into kbn-59189-recursi…
pgayvallet Jun 26, 2020
c05880a
Merge remote-tracking branch 'upstream/master' into kbn-59189-recursi…
pgayvallet Jun 29, 2020
9dfbf16
remove `flat` usage
pgayvallet Jun 29, 2020
9b67058
Merge remote-tracking branch 'upstream/master' into kbn-59189-recursi…
pgayvallet Jun 29, 2020
39f2765
Merge remote-tracking branch 'upstream/master' into kbn-59189-recursi…
pgayvallet Jun 29, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,7 @@
"@types/markdown-it": "^0.0.7",
"@types/minimatch": "^2.0.29",
"@types/mocha": "^7.0.2",
"@types/mock-fs": "^4.10.0",
"@types/moment-timezone": "^0.5.12",
"@types/mustache": "^0.8.31",
"@types/node": ">=10.17.17 <10.20.0",
Expand Down Expand Up @@ -472,6 +473,7 @@
"listr": "^0.14.1",
"load-grunt-config": "^3.0.1",
"mocha": "^7.1.1",
"mock-fs": "^4.12.0",
"mock-http-server": "1.3.0",
"ms-chromium-edge-driver": "^0.2.3",
"multistream": "^2.1.1",
Expand Down
0 ...tures__/mock_repo/plugins/baz/kibana.json → .../mock_repo/plugins/nested/baz/kibana.json
View file
Open in desktop
File renamed without changes.
0 ...s__/mock_repo/plugins/baz/server/index.ts → ...k_repo/plugins/nested/baz/server/index.ts
View file
Open in desktop
File renamed without changes.
0 ...res__/mock_repo/plugins/baz/server/lib.ts → ...ock_repo/plugins/nested/baz/server/lib.ts
View file
Open in desktop
File renamed without changes.
12 changes: 6 additions & 6 deletions packages/kbn-optimizer/src/integration_tests/__snapshots__/basic_optimization.test.ts.snap
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 6 additions & 6 deletions packages/kbn-optimizer/src/optimizer/kibana_platform_plugins.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,18 +41,18 @@ it('parses kibana.json files of plugins found in pluginDirs', () => {
"id": "bar",
"isUiPlugin": true,
},
Object {
"directory": <absolute path>/packages/kbn-optimizer/src/__fixtures__/mock_repo/plugins/baz,
"extraPublicDirs": Array [],
"id": "baz",
"isUiPlugin": false,
},
Object {
"directory": <absolute path>/packages/kbn-optimizer/src/__fixtures__/mock_repo/plugins/foo,
"extraPublicDirs": Array [],
"id": "foo",
"isUiPlugin": true,
},
Object {
"directory": <absolute path>/packages/kbn-optimizer/src/__fixtures__/mock_repo/plugins/nested/baz,
"extraPublicDirs": Array [],
"id": "baz",
"isUiPlugin": false,
},
Object {
"directory": <absolute path>/packages/kbn-optimizer/src/__fixtures__/mock_repo/test_plugins/test_baz,
"extraPublicDirs": Array [],
Expand Down
2 changes: 1 addition & 1 deletion packages/kbn-optimizer/src/optimizer/kibana_platform_plugins.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export function findKibanaPlatformPlugins(scanDirs: string[], paths: string[]) {
.sync(
Array.from(
new Set([
...scanDirs.map((dir) => `${dir}/*/kibana.json`),
...scanDirs.map((dir) => `${dir}/**/kibana.json`),
Copy link
Contributor Author

@pgayvallet pgayvallet Jun 10, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Comparing this diff with the changes in plugins_discovery.ts made me cry a little.

Note that the optimizer does not exactly reproduce the logic used in the discovery system:

  • It doesn't handle maxDepth
  • It will discover plugins nested inside other plugins

However, I think this is acceptable. Worse case some plugins will be build that are not going to be discovered by core's discovery system. But that would be detected during the development phase anyway. (and implementing the exact same logic would cause findKibanaPlatformPlugins to become way more complex than its currently is)

Copy link
Contributor

Choose a reason for hiding this comment

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

Wonder if we could have some sort of maxDepth by using a (very unreadable) glob like:

`${dir}/{*,*/*,*/*/*,*/*/*/*,*/*/*/*/*}/kibana.json`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Either that or

 ...scanDirs
            .map((dir) => [
              `${dir}/*/kibana.json`,
              `${dir}/*/*/kibana.json`,
              `${dir}/*/*/*/kibana.json`,
              `${dir}/*/*/*/*/kibana.json`,
              `${dir}/*/*/*/*/*/kibana.json`,
            ])
            .flat(),

Which is a little longer (even if it can be extracted to a function), but probably more explicit.

Not 100% sure this is really needed (as it doesn't address the second point), but I have no problem implementing that if we think it's a little better can current implem.

wdyt?

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's err on the side of caution here and go ahead and do that. Optimizer is already slow as it is, let's be defensive.

...paths.map((path) => `${path}/kibana.json`),
])
),
Expand Down
8 changes: 8 additions & 0 deletions renovate.json5
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -636,6 +636,14 @@
'(\\b|_)mocha(\\b|_)',
],
},
{
groupSlug: 'mock-fs',
groupName: 'mock-fs related packages',
packageNames: [
'mock-fs',
'@types/mock-fs',
],
},
{
groupSlug: 'moment',
groupName: 'moment related packages',
Expand Down
9 changes: 0 additions & 9 deletions src/core/server/plugins/discovery/plugins_discovery.test.mocks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,5 @@
* under the License.
*/

export const mockReaddir = jest.fn();
export const mockReadFile = jest.fn();
export const mockStat = jest.fn();
jest.mock('fs', () => ({
readdir: mockReaddir,
readFile: mockReadFile,
stat: mockStat,
}));

export const mockPackage = new Proxy({ raw: {} as any }, { get: (obj, prop) => obj.raw[prop] });
jest.mock('../../../../../package.json', () => mockPackage);
Loading