[Remote clusters] Migrate client-side code out of legacy#57365
[Remote clusters] Migrate client-side code out of legacy#57365alisonelizabeth merged 13 commits intoelastic:masterfrom
Conversation
|
Pinging @elastic/es-ui (Team:Elasticsearch UI) |
| ({ component, form, actions } = setup()); | ||
|
|
||
| await nextTick(); | ||
| await nextTick(100); |
There was a problem hiding this comment.
I needed to adjust the timeout when switching from axios to the http service
There was a problem hiding this comment.
Was the issue the same in each instance where nextTick was added? Just wondering whether it is worth putting a comment in each time.
Not a big deal though :)
There was a problem hiding this comment.
Good point! I may have gotten carried away 😂. It actually is not needed for this specific line, but I updated the code where it was needed with a comment.
| } = core; | ||
|
|
||
| // Initialize services | ||
| initBreadcrumbs(setBreadcrumbs); |
There was a problem hiding this comment.
@jloleysens This is the type of convention I was trying to get to. In my apps I am migrating I am instantiating the service inside the plugin setup() cycle.
There was a problem hiding this comment.
👍 I guess in this pattern we would also not want the stateful services to be accessed via import in downstream code (as it currently is), e.g.,
More asking 👆🏻 :)
There was a problem hiding this comment.
Correct, these are still being accessed via import. I was on the fence to whether it made sense to refactor as part of this migration PR. I think it might be better to open up a separate PR once this is merged and move to context if that is the pattern we want to follow.
jloleysens
left a comment
There was a problem hiding this comment.
These changes look great to me! Always good to see a PR that is "in the red" 😉
Left a question regarding tests, not a blocker.
| ({ component, form, actions } = setup()); | ||
|
|
||
| await nextTick(); | ||
| await nextTick(100); |
There was a problem hiding this comment.
Was the issue the same in each instance where nextTick was added? Just wondering whether it is worth putting a comment in each time.
Not a big deal though :)
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
…re/files-and-filetree * 'master' of github.com:elastic/kibana: (139 commits) Move Ace XJSON lexer-rules, worker and utils to es_ui_shared (elastic#57563) [Upgrade Assistant] Fix filter deprecations search filter (elastic#57541) [ML] New Platform server shim: update indices routes (elastic#57685) Bump redux dependencies (elastic#53348) [Index management] Client-side NP ready (elastic#57295) change id of x-pack event_log plugin to eventLog (elastic#57612) [eventLog] get kibana.index name from config instead of hard-coding it (elastic#57607) revert allow using any path to generate fixes ui titles (elastic#57535) Fix login redirect for expired sessions (elastic#57157) Expose Vis on the contract as it requires visTypes (elastic#56968) [SIEM][Detection Engine] Fixes queries to ignore errors when signals index is not present [Remote clusters] Migrate client-side code out of legacy (elastic#57365) Fix failed test reporter for SIEM Cypress use (elastic#57240) skip flaky suite (elastic#45244) update chromedriver to 80.0.1 (elastic#57602) change slack action to only report on whitelisted host name (elastic#57582) [kbn/optimizer] throw errors into stream on invalid completion (elastic#57735) moving visualize/utils to new platform (elastic#56650) ...
This PR mostly completes the NP migration for the
remote_clustersplugin.Continuation of #56781.
What's left
How to test
You will need to set up two clusters in order to test remote clusters. For example:
xpack.remote_clusters.enabledisfalse