[Ingest pipelines] Add copy_from to set processor #104070

sabarasaba · 2021-07-01T10:06:48Z

Fixes: #101246

As a side note, either value or copy_from can be defined at any given time, not both.

Release Note

The Ingest Node Pipelines UI added support to configure a copy_from field on the set processor.

Default description

New processor fields

sabarasaba · 2021-07-05T11:43:43Z

@elasticmachine merge upstream

sabarasaba · 2021-07-13T09:50:39Z

@elasticmachine merge upstream

kibanamachine · 2021-07-13T09:50:40Z

merge conflict between base and head

sabarasaba · 2021-07-16T08:55:08Z

@elasticmachine merge upstream

alisonelizabeth

Nice work @sabarasaba!

Not sure if this one was ready for review yet (still in draft), but since I was tagged thought I'd give it a quick look as I'll be OOO next week 😄.

I left a few comments in the code. Overall, everything worked as expected.

I'm wondering if this one merits some more UX consideration. For example, what if there was first a selection - do you want to set a string value, or copy from an existing field? Then, there would just be one text input displayed based on the selection. I guess this would require an extra step for the user, so not sure if this is the best approach or not. However, I can see it being beneficial to better associate these two fields together so it's more clear that it's a one-or-another type deal. Would you mind reaching out to @dborodyansky to get his thoughts?

Also, I think this could benefit from a copy review. I think it would be helpful if we could better distinguish why a user might want to use value over copy_from, or vice versa.

...s/public/application/components/pipeline_editor/components/processor_form/processors/set.tsx

...est_pipelines/public/application/components/pipeline_editor/__jest__/processors/set.test.tsx

...s/public/application/components/pipeline_editor/components/processor_form/processors/set.tsx

sabarasaba · 2021-07-19T07:10:32Z

@alisonelizabeth Sorry you got pinged early, didnt thought gh will do when the PR was still in draft mode! Thanks for checking it out, will reach out to Dmitry and will ask the docs team to have a look later :)

sabarasaba · 2021-07-19T07:11:11Z

@elasticmachine merge upstream

elasticmachine · 2021-07-19T07:11:31Z

Pinging @elastic/kibana-stack-management (Team:Stack Management)

sabarasaba · 2021-07-20T07:14:06Z

After a small discussion with @dborodyansky, we agreed on following a similar pattern as we used in the Network Direction processor and allow users to toggle between them two mutually exclusive fields:

jrodewig

Thanks @sabarasaba. I left some non-blocking suggestions that you can ignore if wanted.

Thanks for working on this.

...s/public/application/components/pipeline_editor/components/processor_form/processors/set.tsx

alisonelizabeth

Nice work @sabarasaba! UX changes look good 👍 .

I left a couple nits in the code. I'd like to see at least the validation messages addressed before merging.

I also noticed the spacing is different with the toggle field. However, I also see the same issue with the network_direction processor.

...est_pipelines/public/application/components/pipeline_editor/__jest__/processors/set.test.tsx

...s/public/application/components/pipeline_editor/components/processor_form/processors/set.tsx

sabarasaba · 2021-07-27T06:34:33Z

@elasticmachine merge upstream

kibanamachine · 2021-07-27T06:34:35Z

merge conflict between base and head

sabarasaba · 2021-07-27T12:34:57Z

Thanks again for re-checking @alisonelizabeth! Nice spot on the labels with toggle links not being exactly the same as the other fields. I've addressed this with d2b795e for both processors:

kibanamachine · 2021-07-27T14:48:47Z

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`ingestPipelines`	706.0KB	709.7KB	+3.7KB

History

💚 Build #140922 succeeded 77e205b
💚 Build #139728 succeeded ccf8007
💚 Build #139625 succeeded 03e5112
💔 Build #139267 failed 4a8e92b
💚 Build #138992 succeeded a367a04

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

cc @sabarasaba

* fix up validation conditions for fields * add tests and fix prettier errors * Small refactor and fix tests * Fix copy surrounding error handling * Clean up unnecessary boilerplate from tests * Fix i18n error * Keep optional select next to bound input * Pass disabled prop as boolean * fix test matchers * No need to whitelist fields anymore * Small CR changes * Convert optional inputs to toggle state * Fix testcase copy * address CR suggestions * address CR changes * Fix i18n * Fix labelAppend link alignment # Conflicts: # x-pack/plugins/translations/translations/ja-JP.json # x-pack/plugins/translations/translations/zh-CN.json

* fix up validation conditions for fields * add tests and fix prettier errors * Small refactor and fix tests * Fix copy surrounding error handling * Clean up unnecessary boilerplate from tests * Fix i18n error * Keep optional select next to bound input * Pass disabled prop as boolean * fix test matchers * No need to whitelist fields anymore * Small CR changes * Convert optional inputs to toggle state * Fix testcase copy * address CR suggestions * address CR changes * Fix i18n * Fix labelAppend link alignment

sabarasaba force-pushed the add_copy_from_to_set_processor branch from 4393c5d to ed7527f Compare July 1, 2021 11:44

sabarasaba self-assigned this Jul 5, 2021

sabarasaba added Feature:Ingest Node Pipelines Ingest node pipelines management release_note:feature Makes this part of the condensed release notes Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v7.15.0 v8.0.0 labels Jul 13, 2021

sabarasaba requested a review from alisonelizabeth July 16, 2021 09:15

alisonelizabeth reviewed Jul 16, 2021

View reviewed changes

sabarasaba marked this pull request as ready for review July 19, 2021 07:11

sabarasaba requested a review from a team as a code owner July 19, 2021 07:11

jrodewig approved these changes Jul 20, 2021

View reviewed changes

alisonelizabeth approved these changes Jul 26, 2021

View reviewed changes

sabarasaba added 7 commits July 27, 2021 09:41

fix up validation conditions for fields

60b3efd

add tests and fix prettier errors

9a69b54

Small refactor and fix tests

2477a9f

Fix copy surrounding error handling

e43134e

Clean up unnecessary boilerplate from tests

9a69ab3

Fix i18n error

096099b

Keep optional select next to bound input

e0fcf9f

sabarasaba added 9 commits July 27, 2021 09:43

Pass disabled prop as boolean

619ad6e

fix test matchers

2694529

No need to whitelist fields anymore

8082da0

Small CR changes

29113ac

Convert optional inputs to toggle state

e968219

Fix testcase copy

af8c41a

address CR suggestions

e52067a

address CR changes

f98c1fe

Fix i18n

77e205b

sabarasaba force-pushed the add_copy_from_to_set_processor branch from 94bbf31 to 77e205b Compare July 27, 2021 07:32

Fix labelAppend link alignment

d2b795e

sabarasaba merged commit 374de9d into elastic:master Jul 27, 2021

sabarasaba mentioned this pull request Jul 28, 2021

[7.x] [Ingest pipelines] Add copy_from to set processor (#104070) #106953

Merged

yuliacech mentioned this pull request Sep 7, 2021

[ILM] Added max_primary_shard_size input to shrink action #111394

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Ingest pipelines] Add copy_from to set processor #104070

[Ingest pipelines] Add copy_from to set processor #104070

sabarasaba commented Jul 1, 2021 •

edited

sabarasaba commented Jul 5, 2021

sabarasaba commented Jul 13, 2021

kibanamachine commented Jul 13, 2021

sabarasaba commented Jul 16, 2021

alisonelizabeth left a comment

sabarasaba commented Jul 19, 2021

sabarasaba commented Jul 19, 2021

elasticmachine commented Jul 19, 2021

sabarasaba commented Jul 20, 2021

jrodewig left a comment

alisonelizabeth left a comment

sabarasaba commented Jul 27, 2021

kibanamachine commented Jul 27, 2021

sabarasaba commented Jul 27, 2021

kibanamachine commented Jul 27, 2021

[Ingest pipelines] Add copy_from to set processor #104070

[Ingest pipelines] Add copy_from to set processor #104070

Conversation

sabarasaba commented Jul 1, 2021 • edited

Release Note

Default description

New processor fields

sabarasaba commented Jul 5, 2021

sabarasaba commented Jul 13, 2021

kibanamachine commented Jul 13, 2021

sabarasaba commented Jul 16, 2021

alisonelizabeth left a comment

Choose a reason for hiding this comment

sabarasaba commented Jul 19, 2021

sabarasaba commented Jul 19, 2021

elasticmachine commented Jul 19, 2021

sabarasaba commented Jul 20, 2021

jrodewig left a comment

Choose a reason for hiding this comment

alisonelizabeth left a comment

Choose a reason for hiding this comment

sabarasaba commented Jul 27, 2021

kibanamachine commented Jul 27, 2021

sabarasaba commented Jul 27, 2021

kibanamachine commented Jul 27, 2021

💚 Build Succeeded

Metrics [docs]

Async chunks

History

sabarasaba commented Jul 1, 2021 •

edited