Skip to content

[ILM] TS conversion of Index Management plugin extension#76308

Closed
yuliacech wants to merge 26 commits intoelastic:masterfrom
yuliacech:ilm_index_management
Closed

[ILM] TS conversion of Index Management plugin extension#76308
yuliacech wants to merge 26 commits intoelastic:masterfrom
yuliacech:ilm_index_management

Conversation

@yuliacech
Copy link
Copy Markdown
Contributor

@yuliacech yuliacech commented Aug 31, 2020

Summary

This PR converts Index Management plugin extension to TypeScript as an ongoing effort of converting the ILM plugin #74271. No UI/UX changes were made in this PR. To test for any regression, navigate to Index Management plugin and check following features in the Indices tab:

  • Action button to retry a lifecycle policy (only shown if all indices in the list have a lifecycle phase error)
  • Action button to add a lifecycle policy (if the index is not managed already)
  • Action button to delete a lifecycle policy (if the index is managed)
  • A banner on top of the indices list if there is an index with a lifecycle phase error (an error can be created by setting non existent snapshot policy name in Delete phase, for example)
  • A section with lifecycle policy information in the Index details flyout (only shown if the index is managed)
  • A filter for lifecycle status and lifecycle phase

Todo

After this PR and policies table conversion PR #76006 are both merged, a follow-up PR will convert all the remaining js files, such as jest tests, services and helpers, also any remaining any types will be cleaned up to complete #74271

yuliacech and others added 3 commits August 31, 2020 18:55
* eui to v28.0.0

* eui to 28.2.0

* euiselectableoptions type update

* targeted nohoist

* src snapshot updates

* x-pack snapshot updates

* strong -> mark

* upgrade @elastic/charts to v21.0.1

* strong -> mark

* fix charts version merge

* maps -> add_field_tooltip_popover type update

* snapshot update

* Fix gridline visibility

Co-authored-by: Justin Kambic <justin.kambic@elastic.co>

Co-authored-by: nickofthyme <nick.ryan.partridge@gmail.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Co-authored-by: Justin Kambic <justin.kambic@elastic.co>
@yuliacech yuliacech marked this pull request as ready for review August 31, 2020 17:19
@yuliacech yuliacech requested a review from a team as a code owner August 31, 2020 17:19
@yuliacech yuliacech added chore Feature:ILM release_note:skip Skip the PR/issue when compiling release notes Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more t// v7.10.0 v8.0.0 labels Aug 31, 2020
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/es-ui (Team:Elasticsearch UI)

patrykkopycinski and others added 8 commits August 31, 2020 19:30
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Co-authored-by: spalger <spalger@users.noreply.github.com>
…#76278)

* fix URI decoding and editing of a policy which backs up all indices

* fix type issue

* fix general use of encoding and update decode algo

* fix serialisation of snapshots and added a test

* Fix test description name

* Update attempt_to_uri_decode.ts

* catch errors from decoding in the already throwing code

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
* Reconcile request helpers with eslint rules for React hooks.
  - Add clearer cleanup logic for unmounted components.
  - Align logic and comments in np_ready_request.ts and original request.ts.
* Reorganize modules and convert tests to TS.
  - Split request.ts into send_request.ts and use_request.ts.
  - Convert test files into TS.
  - Relax SendRequestResponse type definition to type error as any instead of expecting an Error, since we weren't actually fulfilling this expectation.
* Convert everything to hooks and add test coverage for request behavior.
* Fix Watcher memoization bugs.
Copy link
Copy Markdown
Contributor

@alisonelizabeth alisonelizabeth left a comment

Choose a reason for hiding this comment

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

Great work @yuliacech! I tested locally and did not find any regressions.

I left a few code comments. Also, regarding the use of any for indices - could you utilize this interface in index_management: https://github.com/elastic/kibana/blob/master/x-pack/plugins/index_management/server/types.ts#L29?

selectedPolicyName: string;
selectedAlias: string;
policies: PolicyFromES[];
policyError?: string;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: might be more clear as policyErrorMessage. I know this was pre-existing, so feel free to ignore 😄

</EuiDescriptionListDescription>,
];
const cell = (
<Fragment>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: you can also use <></> for fragments

};

export const removeLifecyclePolicyActionExtension = ({ indices, reloadIndices }) => {
export const removeLifecyclePolicyActionExtension = ({ indices, reloadIndices }: any) => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

At the very least, I think this could be improved to:

Suggested change
export const removeLifecyclePolicyActionExtension = ({ indices, reloadIndices }: any) => {
export const removeLifecyclePolicyActionExtension = ({ indices, reloadIndices }: {indices: any; reloadIndices: () => void}) => {

import { RemoveLifecyclePolicyConfirmModal } from './components/remove_lifecycle_confirm_modal';

// eslint-disable-next-line @kbn/eslint/no-restricted-paths
import { ExtensionsSetup } from '../../../index_management/public/services';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think if you export ExtensionsSetup from x-pack/plugins/index_management/public/index.ts you can remove the eslint-disable.

this.setState({ showPhaseExecutionPopover: false });
};
renderStackPopoverButton(ilm) {
renderStackPopoverButton(ilm: any) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can we avoid using any here?

if (!ilm.phase_execution) {
return null;
}
renderPhaseExecutionPopoverButton(ilm: any) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can we avoid using any here?

nreese and others added 8 commits August 31, 2020 14:19
* [Maps] fix duplicate ID's

* tslint cleanup

* use layer id instead of layer name for popover id

* tslint fixes

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
* Add telemetry around workpad variable usage

* Fix typecheck

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
…option. (elastic#76061)

* Exposed separate from ProxySettings rejectUnauthorized configuration option.

* Fixed type checks

* fixed tests
* delete src/legacy/ui/public folder

* remove jest.mock('ui/XXX'); from tests

* adapt stubbedLogstashIndexPatternService

* remove delete keys from translation files

* fix types import with Capabilities

* remove legacy test utils

* remove dead file referencing ui/newPlatform

* move saved-object-finder styles to timelion plugin
mdelapenya and others added 3 commits September 1, 2020 10:10
* chore: group tests doing the same

This will reduce the time it takes to execute the tests

* chore: rephrase step

* chore: extract common code to a function

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
…75537)

* add intergration test for install prebuilt timelines

* add integration tests

* add functional test for export timeline

* clean up

* asserts the content of the exported timeline

* update selector

* update selector

* reuses the timeline id from the expected exported file

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Co-authored-by: Gloria Hornero <snootchie.boochies@gmail.com>
@yuliacech
Copy link
Copy Markdown
Contributor Author

@elasticmachine merge upstream

pgayvallet and others added 3 commits September 1, 2020 12:19
* cleanup xpack_main legacy plugin, remove capabilities mixin

* fix test env

* delete injectXpackSignature and related tests
…ndex_management

# Conflicts:
#	x-pack/plugins/snapshot_restore/public/application/lib/index.ts
@yuliacech yuliacech requested a review from a team September 1, 2020 10:22
@yuliacech yuliacech requested a review from a team as a code owner September 1, 2020 10:22
@yuliacech yuliacech requested a review from a team September 1, 2020 10:22
@yuliacech yuliacech requested a review from a team as a code owner September 1, 2020 10:22
@yuliacech yuliacech requested a review from a team September 1, 2020 10:22
@yuliacech yuliacech requested review from a team as code owners September 1, 2020 10:22
@yuliacech yuliacech requested a review from a team September 1, 2020 10:22
@yuliacech yuliacech requested a review from a team as a code owner September 1, 2020 10:22
@yuliacech
Copy link
Copy Markdown
Contributor Author

@elasticmachine merge upstream

@yuliacech yuliacech closed this Sep 1, 2020
@kibanamachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

Failed CI Steps

Build metrics

‼️ metrics were not reported for [#76308@e6e5939]

History

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore Feature:ILM release_note:skip Skip the PR/issue when compiling release notes Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more t// v7.10.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.