Index patterns api - load field list on server #81218
Index patterns api - load field list on server #81218mattkime merged 42 commits intoelastic:masterfrom
Conversation
…ve_legacy_es_client_usage
…:mattkime/kibana into field_caps_remove_legacy_es_client_usage
…legacy_es_client_usage
…tkime/kibana into single_call_for_index_pattern_fields
|
Pinging @elastic/kibana-app-arch (Team:AppArch) |
| ): (request: KibanaRequest) => Services { | ||
| return (request) => ({ | ||
| callCluster: elasticsearch.legacy.client.asScoped(request).callAsCurrentUser, | ||
| esClient: elasticsearch.client.asScoped(request).asCurrentUser, |
There was a problem hiding this comment.
Should be able to use scopedClusterClient below.
| esClient: elasticsearch.client.asScoped(request).asCurrentUser, |
cjcenizal
left a comment
There was a problem hiding this comment.
Rubberstamping the ES UI codeowner request since the portions of the rollup plugin that were touched aren't owned by us.
💚 Build SucceededMetrics [docs]distributable file count
page load bundle size
History
To update your PR or re-run it, just comment with: |
timroes
left a comment
There was a problem hiding this comment.
Reviewed Kibana App code change only. Code LGTM
Index patterns api - load field list on server
|
Sorry, this caused failures on master once merged so I had to revert. https://kibana-ci.elastic.co/job/elastic+kibana+master/9408/execution/node/459/log |
| asScopedToClient: async ( | ||
| savedObjectsClient: SavedObjectsClientContract, | ||
| elasticsearchClient: ElasticsearchClient | ||
| ) => { |
There was a problem hiding this comment.
@mattkime Just stumbled across this change now -- I'm wondering if it would be simpler to instead do:
asScoped: async (request: KibanaRequest) => {...}Since both the scoped saved objects & es clients can be derived from a kibana request:
const savedObjectsClient = savedObjects.getScopedClient(req);
const elasticsearchClient = elasticsearch.client.asScoped(req);The reason I called this asScopedToClient in the first place was because it mirrored the signature of uiSettings.asScopedToClient which only takes in a scoped SO client. Now that we've changed it, the naming is a bit more confusing IMHO.
In the search service (both low level & search source), we've used asScoped(req: KibanaRequest), so this would also make things consistent with the other search services.
There was a problem hiding this comment.
I thought this was following the general principle of asking for for the more specific dependencies over more general. That said, I'm definitely open to improvements in the code. Mind opening a PR?
There was a problem hiding this comment.
principle of asking for for the more specific dependencies over more general
Yeah, that's a fair point and I agree the whole KibanaRequest might not always be necessary. I guess it's more of a consistency-vs-explicitness tradeoff. I think I was getting hung up on the naming here in particular though; I'll give it some thought.
Summary
The server side index patterns api can now load the field list. Previously it would error if a field list refresh was required.
The regular and rollup index pattern field list loading methods were merged. Rollup index patterns require additional functionality over regular index patterns when it comes to loading the field list, but this won't be necessary once rollups v2 is released.
Checklist
Delete any items that are not applicable to this PR.
For maintainers