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

Ingest Node Pipelines UI #62321

Merged
merged 33 commits into from
Apr 30, 2020
Merged

Ingest Node Pipelines UI #62321

merged 33 commits into from
Apr 30, 2020

Conversation

alisonelizabeth
Copy link
Contributor

@alisonelizabeth alisonelizabeth commented Apr 2, 2020 •
edited
Loading

Summary

This PR delivers the Ingest Node Pipelines UI MVP.

Note to reviewers

All commits have been reviewed via separate PRs that were merged into the feature branch. This PR should only require a sanity check for any glaring errors.

What's left

  • Copy is still being reviewed. I will create a follow-up PR with any changes.
  • Improve test coverage.
  • Accessibility audit. I will create a follow-up PR with any major accessibility concerns.

Release note

A new application called Ingest Node Pipelines is available in Kibana's Management section. It provides a way to manage Elasticsearch's ingest node pipelines. Users can create, edit, clone, delete, and test a pipeline.

Screenshots

Pipeline list

Empty prompt:
Screen Shot 2020-04-29 at 12 46 08 PM

Table:
Screen Shot 2020-04-29 at 12 56 09 PM

Error state:
Screen Shot 2020-04-27 at 9 39 58 PM

Delete modal:
Screen Shot 2020-04-29 at 12 56 18 PM

Details panel:
Screen Shot 2020-04-29 at 12 55 53 PM

Create pipeline

Create form:
create

Form validation errors:
errors

ES request flyout:
Screen Shot 2020-04-29 at 12 52 38 PM

Test flyout (documents tab):
Screen Shot 2020-04-29 at 12 52 47 PM

Test flyout (output tab):
Screen Shot 2020-04-29 at 12 53 23 PM

Error handling:
Screen Shot 2020-04-29 at 12 54 42 PM

Edit pipeline

Edit form:
edit

Clone pipeline

Clone form:
clone

@alisonelizabeth alisonelizabeth added release_note:enhancement v8.0.0 Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v7.8.0 labels Apr 2, 2020
@elasticmachine
Copy link
Contributor

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

@alisonelizabeth alisonelizabeth force-pushed the feature/ingest-node-pipelines branch 2 times, most recently from e1523a2 to 174136e Compare April 3, 2020 18:36
@alisonelizabeth
Copy link
Contributor Author

@elasticmachine merge upstream

@alisonelizabeth
Copy link
Contributor Author

@elasticmachine merge upstream

@alisonelizabeth
Copy link
Contributor Author

@elasticmachine merge upstream

elasticmachine and others added 3 commits April 8, 2020 15:27
@elasticmachine
Merge branch 'master' into feature/ingest-node-pipelines
ac0ac2b
@jloleysens @elasticmachine
List pipelines (#62785)
699ebef 
* First iteration of ingest table

* Add action placeholders

* Refactor list and address PR feedback

Refactored the list into smaller pieces and assemble in main.tsx

Also addressed feedback on copy, removed unused notifications dep

* WiP on flyout

Showing name in title

* Add reload button

* Finish first version of flyout

* Slight update to copy

* `delete` -> `edit`

* Address PR feedback

Copy and a11y updates

* Add on failure JSON to flyout if it is available

* Add details json block file and remove ununsed import

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@alisonelizabeth
[Ingest pipelines] Create pipeline UI (#63017)
92b3f21
@alisonelizabeth
Copy link
Contributor Author

@elasticmachine merge upstream

@elasticmachine
Copy link
Contributor

merge conflict between base and head

@cjcenizal cjcenizal added the Feature:Ingest Node Pipelines Ingest node pipelines management label Apr 14, 2020
@alisonelizabeth
Copy link
Contributor Author

@elasticmachine merge upstream

@elasticmachine
Copy link
Contributor

merge conflict between base and head

@alisonelizabeth alisonelizabeth changed the title Ingest Node Pipelines UI (WIP) Ingest Node Pipelines UI Apr 29, 2020
@alisonelizabeth alisonelizabeth marked this pull request as ready for review April 29, 2020 19:46
@alisonelizabeth alisonelizabeth requested a review from a team as a code owner April 29, 2020 19:46
@alisonelizabeth
Copy link
Contributor Author

@elasticmachine merge upstream

@jloleysens jloleysens mentioned this pull request Apr 30, 2020
Closed
jloleysens
jloleysens approved these changes Apr 30, 2020
View reviewed changes
Copy link
Contributor

@jloleysens jloleysens left a comment

Choose a reason for hiding this comment

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

@alisonelizabeth this is looking in great shape!

Code

I left some nits in the code.

UI/UX

Tested all of the interactions I could think of 😅 and everything is working as expected! Just two very minor comments:

Might be nice to add a doc link to the simulate endpoint here (unless I missed it):

Screenshot 2020-04-30 at 11 12 48

Nit; this button's counterpart (Hide request) will not be clickable (without spidey reflex :p) on create/edit pipeline screen because it is obscured by the flyout.

Screenshot 2020-04-30 at 11 13 49

x-pack/plugins/ingest_pipelines/server/routes/api/update.ts Outdated Show resolved Hide resolved
x-pack/plugins/ingest_pipelines/server/routes/api/simulate.ts Outdated Show resolved Hide resolved
@alisonelizabeth
Copy link
Contributor Author

Thanks @jloleysens for the review! I addressed your feedback.

Nit; this button's counterpart (Hide request) will not be clickable (without spidey reflex :p) on create/edit pipeline screen because it is obscured by the flyout.

Great point 😄! Although not the best experience, I think this is OK for now since the flyout can still be closed via the x at the top. FWIW the "Hide request" can still be seen on large screen sizes. I also see the same problem with the Remote clusters app.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

cjcenizal
cjcenizal approved these changes Apr 30, 2020
View reviewed changes
Copy link
Contributor

@cjcenizal cjcenizal left a comment

Choose a reason for hiding this comment

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

Looked through the UI, looks great! My one suggestion would be to move the app to the top of the Elasticsearch section in the nav, to follow the model of data being ingested first, then managed second.

image

@mdefazio
Copy link
Contributor

LGTM!

@alisonelizabeth
Copy link
Contributor Author

@cjcenizal thanks for the review!

My one suggestion would be to move the app to the top of the Elasticsearch section in the nav, to follow the model of data being ingested first, then managed second.

I'm glad you brought this up! I was thinking about this when I first set up the app, but forgot to follow up. I think it makes sense to put it at the top as well. Looking at the code, it looks like I will have to update the order prop for the other plugins as well. I think I'd prefer to make this change in a separate PR after this is merged.

@alisonelizabeth alisonelizabeth merged commit 6e3791e into master Apr 30, 2020
@alisonelizabeth alisonelizabeth mentioned this pull request Apr 30, 2020
Merged
v1v added a commit to v1v/kibana that referenced this pull request May 4, 2020
@v1v
Merge remote-tracking branch 'upstream/master' into feature/e2e-changes
0dd3b32 
* upstream/master: (315 commits)
  [APM] Fix failing `ApmIndices` test (elastic#64965)
  [APM] Fix paths for ts optimization script (elastic#65012)
  Use HDR for percentiles (elastic#64758)
  [EPM] fix updates available filter (elastic#64957)
  [Uptime] Certificates page (elastic#64059)
  load lens app lazily (elastic#64769)
  [legacy/server/config] remove unnecessary deps for simple helper (elastic#64954)
  Fixed alert Edit flyout shows the error message when one of this actions has a preconfigured action type (elastic#64742)
  [data.search.aggs] Remove legacy aggs APIs. (elastic#64719)
  Fixed `AddAlert` flyout does not immediately update state to reflect new props (elastic#64927)
  [Discover] Show doc viewer action buttons on focus (elastic#64912)
  [EPM] restrict package install endpoint from installing/updating to old packages (elastic#64932)
  [Metrics UI] Add inventory metric threshold alerts (elastic#64292)
  [Canvas] Adds edit menu (elastic#64738)
  [Canvas] Adds refresh and autoplay options to view menu (elastic#64375)
  [Lens] Trigger a filter action on click in datatable visualization (elastic#63840)
  [SIEM][CASE] Refactor Connectors - Jira Connector (elastic#63450)
  [APM] Client new platform migration (elastic#64046)
  [Monitoring] NP Migration complete client cutover (elastic#62908)
  Ingest Node Pipelines UI (elastic#62321)
  ...
jloleysens added a commit to jloleysens/kibana that referenced this pull request May 4, 2020
@jloleysens
Merge branch 'master' of github.com:elastic/kibana into pipeline-edit…
68c12f9 
…or-part-mvp-2

* 'master' of github.com:elastic/kibana: (51 commits)
  [APM] Fix failing `ApmIndices` test (elastic#64965)
  [APM] Fix paths for ts optimization script (elastic#65012)
  Use HDR for percentiles (elastic#64758)
  [EPM] fix updates available filter (elastic#64957)
  [Uptime] Certificates page (elastic#64059)
  load lens app lazily (elastic#64769)
  [legacy/server/config] remove unnecessary deps for simple helper (elastic#64954)
  Fixed alert Edit flyout shows the error message when one of this actions has a preconfigured action type (elastic#64742)
  [data.search.aggs] Remove legacy aggs APIs. (elastic#64719)
  Fixed `AddAlert` flyout does not immediately update state to reflect new props (elastic#64927)
  [Discover] Show doc viewer action buttons on focus (elastic#64912)
  [EPM] restrict package install endpoint from installing/updating to old packages (elastic#64932)
  [Metrics UI] Add inventory metric threshold alerts (elastic#64292)
  [Canvas] Adds edit menu (elastic#64738)
  [Canvas] Adds refresh and autoplay options to view menu (elastic#64375)
  [Lens] Trigger a filter action on click in datatable visualization (elastic#63840)
  [SIEM][CASE] Refactor Connectors - Jira Connector (elastic#63450)
  [APM] Client new platform migration (elastic#64046)
  [Monitoring] NP Migration complete client cutover (elastic#62908)
  Ingest Node Pipelines UI (elastic#62321)
  ...

# Conflicts:
#	x-pack/plugins/ingest_pipelines/common/types.ts
#	x-pack/plugins/ingest_pipelines/public/application/components/pipeline_form/pipeline_form.tsx
#	x-pack/plugins/ingest_pipelines/public/application/components/pipeline_form/pipeline_form_fields.tsx
#	x-pack/plugins/ingest_pipelines/public/shared_imports.ts
@alisonelizabeth alisonelizabeth mentioned this pull request May 4, 2020
Merged
@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

@kibanamachine kibanamachine added backport missing Added to PRs automatically when the are determined to be missing a backport. and removed backport missing Added to PRs automatically when the are determined to be missing a backport. labels May 4, 2020
alisonelizabeth added a commit that referenced this pull request May 5, 2020
@alisonelizabeth
[7.x] Ingest Node Pipelines UI (#62321) (#64956)
2525d57
@alisonelizabeth alisonelizabeth deleted the feature/ingest-node-pipelines branch May 11, 2020 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cjcenizal cjcenizal cjcenizal approved these changes

@mdefazio mdefazio mdefazio approved these changes

@jloleysens jloleysens jloleysens approved these changes

Assignees
No one assigned
Labels
Feature:Ingest Node Pipelines Ingest node pipelines management release_note:enhancement Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v7.8.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@alisonelizabeth @elasticmachine @kibanamachine @mdefazio @cjcenizal @jloleysens