Migrate from Joi to @kbn/config-schema in "home" and "features" plugins #100201

mshustov · 2021-05-17T10:50:48Z

Summary

Migrates joi usages to @kbn/config-schema in home and features plugins

Checklist

Unit or functional tests were updated or added to match the most common scenarios
This was checked for breaking API changes and was labeled appropriately

mshustov · 2021-05-17T14:11:02Z

src/plugins/home/server/services/sample_data/lib/sample_dataset_schema.ts

-  currentTimeMarker: Joi.string().isoDate().required(),
+  currentTimeMarker: schema.string({
+    validate(value: string) {
+      if (isNaN(Date.parse(value))) {


Maybe it's a valid case to extend @kbn/config-schema with date type

It does. I'm surprised we didn't get any feedback about this type missing to be honest.

I will create an issue

created #100635

mshustov · 2021-05-17T14:24:24Z

src/plugins/home/server/services/sample_data/routes/install.ts

        createResults = await client.bulkCreate(
-          sampleDataset.savedObjects.map(({ version, ...savedObject }) => savedObject),
+          savedObjects.map(({ version, ...savedObject }) => savedObject),


@kbn/config-schema creates a result type for { property: schema.any() }, as {property?: any}. That's okay, since any includes undefined or null types, but it's not compatible with savedObject.attributes property,(which is a generic, with unknown default).

mshustov · 2021-05-17T14:26:16Z

x-pack/plugins/apm/server/plugin.ts

@@ -108,6 +108,7 @@ export class APMPlugin
    plugins.home?.tutorials.registerTutorial(() => {
      const ossPart = ossTutorialProvider({});
      if (this.currentConfig!['xpack.apm.ui.enabled'] && ossPart.artifacts) {
+        // @ts-expect-error ossPart.artifacts.application is readonly


Please, consider returning a new object instead of mutating the existing one.

pgayvallet

LGTM

src/plugins/home/server/services/sample_data/lib/sample_dataset_schema.ts

pgayvallet · 2021-05-17T17:50:29Z

src/plugins/home/server/services/sample_data/lib/sample_dataset_schema.ts

-  currentTimeMarker: Joi.string().isoDate().required(),
+  currentTimeMarker: schema.string({
+    validate(value: string) {
+      if (isNaN(Date.parse(value))) {


It does. I'm surprised we didn't get any feedback about this type missing to be honest.

src/plugins/home/server/services/tutorials/lib/tutorial_schema.ts

pgayvallet · 2021-05-17T18:15:34Z

src/plugins/home/server/services/tutorials/lib/tutorial_schema.ts

-
+export type InstructionsSchema = TypeOf<typeof instructionSchema>;
+
+const tutorialIdRegExp = /^[a-zA-Z0-9-]+$/;


Of course it's not the same regexp used everywhere

src/plugins/home/server/services/tutorials/lib/tutorial_schema.ts

elasticmachine · 2021-05-17T18:32:14Z

Pinging @elastic/apm-ui (Team:apm)

x-pack/plugins/features/server/feature_schema.ts

stratoula

Code review only, kibana app code changes LGTM!

kibanamachine · 2021-05-18T10:33:10Z

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`home`	67	64	-3

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
`home`	8	4	-4

Unknown metric groups

API count

id	before	after	diff
`home`	91	88	-3

References to deprecated APIs

id	before	after	diff
`canvas`	29	25	-4
`crossClusterReplication`	8	6	-2
`fleet`	22	20	-2
`globalSearch`	4	2	-2
`indexManagement`	12	7	-5
`infra`	261	149	-112
`lens`	67	45	-22
`licensing`	18	15	-3
`maps`	286	208	-78
`ml`	121	115	-6
`monitoring`	109	56	-53
`stackAlerts`	101	95	-6
total			-295

History

💛 Build #126261 was flaky 2bf8351
💔 Build #126236 failed 49fc22f
💚 Build #126191 succeeded 1036e6f

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

…ns (elastic#100201) * add a link for issue to remove circular deps * features: migrate from joi to config-schema * update tests * migrate home tutorials to config-schema * migrate home dataset validation to config schema * remove unnecessary type. we cannot guarantee this is a valid SO * address Pierres comments

kibanamachine · 2021-05-18T10:36:05Z

💚 Backport successful

Status	Branch	Result
✅	7.x

This backport PR will be merged automatically after passing CI.

…ns (#100201) (#100254) * add a link for issue to remove circular deps * features: migrate from joi to config-schema * update tests * migrate home tutorials to config-schema * migrate home dataset validation to config schema * remove unnecessary type. we cannot guarantee this is a valid SO * address Pierres comments Co-authored-by: Mikhail Shustov <restrry@gmail.com>

…ns (elastic#100201) * add a link for issue to remove circular deps * features: migrate from joi to config-schema * update tests * migrate home tutorials to config-schema * migrate home dataset validation to config schema * remove unnecessary type. we cannot guarantee this is a valid SO * address Pierres comments

mshustov added 3 commits May 17, 2021 12:16

add a link for issue to remove circular deps

7711fa8

features: migrate from joi to config-schema

a330a36

update tests

1036e6f

mshustov added chore v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.14.0 labels May 17, 2021

mshustov added 3 commits May 17, 2021 14:03

migrate home tutorials to config-schema

bb5386e

migrate home dataset validation to config schema

57dd684

remove unnecessary type. we cannot guarantee this is a valid SO

49fc22f

mshustov commented May 17, 2021

View reviewed changes

Merge branch 'master' into issue-61100

2bf8351

mshustov marked this pull request as ready for review May 17, 2021 15:49

mshustov requested a review from a team as a code owner May 17, 2021 15:49

mshustov requested a review from a team May 17, 2021 15:49

mshustov requested a review from a team as a code owner May 17, 2021 15:49

pgayvallet approved these changes May 17, 2021

View reviewed changes

botelastic bot added the Team:APM All issues that need APM UI Team support label May 17, 2021

mshustov commented May 17, 2021

View reviewed changes

x-pack/plugins/features/server/feature_schema.ts Outdated Show resolved Hide resolved

stratoula approved these changes May 18, 2021

View reviewed changes

mshustov added 2 commits May 18, 2021 09:50

Merge branch 'master' into issue-61100

78e6414

address Pierres comments

babc8ce

mshustov enabled auto-merge (squash) May 18, 2021 08:59

pgayvallet mentioned this pull request May 18, 2021

[Cases] RBAC #95058

Merged

mshustov added the auto-backport Deprecated - use backport:version if exact versions are needed label May 18, 2021

dgieselaar approved these changes May 18, 2021

View reviewed changes

mshustov merged commit 574f659 into elastic:master May 18, 2021

kibanamachine mentioned this pull request May 18, 2021

[7.x] Migrate from Joi to @kbn/config-schema in "home" and "features" plugins (#100201) #100254

Merged

mshustov deleted the issue-61100 branch May 18, 2021 12:08

mshustov mentioned this pull request May 26, 2021

@kbn/config-schema should support date types #100635

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate from Joi to @kbn/config-schema in "home" and "features" plugins #100201

Migrate from Joi to @kbn/config-schema in "home" and "features" plugins #100201

mshustov commented May 17, 2021 •

edited

Loading

mshustov May 17, 2021

pgayvallet May 17, 2021

mshustov May 18, 2021

mshustov May 26, 2021

mshustov May 17, 2021

mshustov May 17, 2021

pgayvallet left a comment

pgayvallet May 17, 2021

pgayvallet May 17, 2021

elasticmachine commented May 17, 2021

stratoula left a comment

kibanamachine commented May 18, 2021

API count

References to deprecated APIs

kibanamachine commented May 18, 2021


		export type InstructionsSchema = TypeOf<typeof instructionSchema>;

		const tutorialIdRegExp = /^[a-zA-Z0-9-]+$/;

Migrate from Joi to @kbn/config-schema in "home" and "features" plugins #100201

Migrate from Joi to @kbn/config-schema in "home" and "features" plugins #100201

Conversation

mshustov commented May 17, 2021 • edited Loading

Summary

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pgayvallet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented May 17, 2021

stratoula left a comment

Choose a reason for hiding this comment

kibanamachine commented May 18, 2021

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Public APIs missing exports

API count

References to deprecated APIs

History

kibanamachine commented May 18, 2021

💚 Backport successful

mshustov commented May 17, 2021 •

edited

Loading