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

[Deprecations service] add deprecationType and use it in configs deprecations #100983

Merged
merged 6 commits into from
Jun 1, 2021

Conversation

Bamieh
Copy link
Member

@Bamieh Bamieh commented May 31, 2021

Summary

Ada a new field deprecationType. The field is used to identify between different deprecation types. Example use case: in Upgrade Assistant, we may want to allow the user to sort by deprecation type or show each type in a separate tab.

Current TS definition of the field:

deprecationType?: 'config' | 'feature';

The deprecationType values are predefined to reduce having similar definitions with different keywords across kibana deprecations. Allowing any string to be used as a deprecationType would reduce the usefulness of this field.

The new field is optional. UA might need to categorize deprecations with no deprecationType specified to an uncategorized grouping.

The field is optional to:

  1. not all deprecations fall into a general category of types; we can introduce uncategorized as a dedicated category but that would be almost identical to making the field optional.
  2. reduce the amount of change required to introduce this feature.

Changes

  1. Add deprecationType to deprecations service external contract DeprecationsDetails.
  2. Set deprecationType: 'config' for configs deprecations.

Closes #96060

@Bamieh Bamieh added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.14.0 auto-backport Deprecated - use backport:version if exact versions are needed labels May 31, 2021
@Bamieh Bamieh marked this pull request as ready for review May 31, 2021 12:36
@Bamieh Bamieh requested a review from a team as a code owner May 31, 2021 12:36
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-core (Team:Core)

Copy link
Contributor

@pgayvallet pgayvallet left a comment

Choose a reason for hiding this comment

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

Implementation looks fine. A few NITs and comment on the tests

src/core/server/deprecations/deprecations_service.ts Outdated Show resolved Hide resolved
src/core/server/deprecations/deprecations_service.test.ts Outdated Show resolved Hide resolved
src/core/server/deprecations/deprecations_service.test.ts Outdated Show resolved Hide resolved
src/core/server/deprecations/deprecations_service.test.ts Outdated Show resolved Hide resolved
* Side Public License, v 1.
*/

/* eslint-disable dot-notation */
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is it required?

Copy link
Member Author

Choose a reason for hiding this comment

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

I have a test suite #registerConfigDeprecationsInfo which is a private method

Copy link
Contributor

Choose a reason for hiding this comment

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

Method method = TargetClass.getDeclaredMethod(methodName, argClasses);
method.setAccessible(true);
method.invoke(targetObject, argObjects);

:trollface:

src/core/server/deprecations/deprecations_service.test.ts Outdated Show resolved Hide resolved
@Bamieh Bamieh requested a review from pgayvallet June 1, 2021 10:31
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Unknown metric groups

API count

id before after diff
core 2281 2282 +1

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
lists 239 236 -3
maps 286 208 -78
ml 121 115 -6
monitoring 109 56 -53
securitySolution 390 346 -44
stackAlerts 101 95 -6
total -342

History

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

@Bamieh Bamieh merged commit 1168e11 into elastic:master Jun 1, 2021
@Bamieh Bamieh deleted the deprecations/new_fields branch June 1, 2021 11:09
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jun 1, 2021
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
7.x

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Jun 1, 2021
…precations (#100983) (#101039)

Co-authored-by: Ahmad Bamieh <ahmadbamieh@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v7.14.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Core deprecations service] Add deprecationType field
4 participants