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

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

Closed
wants to merge 26 commits into from

Conversation

yuliacech
Copy link
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 <[email protected]>

Co-authored-by: nickofthyme <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: Justin Kambic <[email protected]>
@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 v7.10.0 v8.0.0 labels Aug 31, 2020
@elasticmachine
Copy link
Contributor

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

patrykkopycinski and others added 8 commits August 31, 2020 19:30
…#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 <[email protected]>
* 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
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
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
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
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 { retryLifecycleForIndex } from '../application/services/api';
import { IndexLifecycleSummary } from './components/index_lifecycle_summary';
import { AddLifecyclePolicyConfirmModal } from './components/add_lifecycle_confirm_modal';
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
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.

@@ -80,7 +98,7 @@ export class IndexLifecycleSummary extends Component {
closePhaseExecutionPopover = () => {
this.setState({ showPhaseExecutionPopover: false });
};
renderStackPopoverButton(ilm) {
renderStackPopoverButton(ilm: any) {
Copy link
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
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 <[email protected]>
* Add telemetry around workpad variable usage

* Fix typecheck

Co-authored-by: Elastic Machine <[email protected]>
…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 <[email protected]>
…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 <[email protected]>
Co-authored-by: Gloria Hornero <[email protected]>
@yuliacech
Copy link
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 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 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
Contributor Author

@elasticmachine merge upstream

@yuliacech yuliacech closed this Sep 1, 2020
@kibanamachine
Copy link
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 v7.10.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.