[Ingest Node Pipelines] Refactor pipeline simulator code#72328
[Ingest Node Pipelines] Refactor pipeline simulator code#72328alisonelizabeth merged 8 commits intoelastic:masterfrom
Conversation
|
Pinging @elastic/es-ui (Team:Elasticsearch UI) |
|
@elasticmachine merge upstream |
|
@elasticmachine merge upstream |
💚 Build SucceededBuild metrics@kbn/optimizer bundle module count
async chunks size
page load bundle size
History
To update your PR or re-run it, just comment with: |
jloleysens
left a comment
There was a problem hiding this comment.
Overall refactor looks good. Great work @alisonelizabeth !
Left one comment regarding the new context props that would be great to get your thoughts on, but considering it non-blocking for this work.
Tested locally and did not spot any regressions 🍻
| api={services.api} | ||
| toasts={services.notifications.toasts} |
There was a problem hiding this comment.
I see we are using the Kibana context provider here. I think it would be cool to add that as a dependency for the processors editor component. So in tests it would become:
<KibanaContextProvider>
<ProcessorsEditorContextProvider>
{/* Editors in here */}
</ProcessorsEditorContextProvider>
</KibanaContextProvider>
That way we can get rid of the api and toasts prop. What do you think? Happy to do this in a separate pass if you agree.
There was a problem hiding this comment.
Great point. I agree. I'd like to get this PR in since it's needed for any subsequent PRs related to the pipeline simulation, but will definitely address in the next pass.
|
|
||
| <PipelineProcessorsContextProvider | ||
| <ProcessorsEditorContextProvider | ||
| onFlyoutOpen={onEditorFlyoutOpen} |
There was a problem hiding this comment.
Using the global flyout will also enable us to remove this prop which would be great!
There was a problem hiding this comment.
Yes! I was thinking the same.
* master: update docs (elastic#74364) [Ingest Node Pipelines] Refactor pipeline simulator code (elastic#72328) Add README files for Kibana app plugins (elastic#74277) [APM] Average for transaction error rate includes null values (elastic#74345) [Ingest Manager] Adjust dataset aggs to use datastream fields instead (elastic#74342)
|
Friendly reminder: Looks like this PR hasn’t been backported yet. |
|
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. |
This PR is prep work for the enhancements planned for the “Test pipeline” workflow.
Changes include:
pipeline_processors_editordirectoryprocessorsandon_failureprocessors to the simulate API; I don't see any benefit to sending the other pipeline fields (version, description).Future polish
How to test
No functionality should have changed here. You can follow the instructions on #64223 to verify there are no regressions.