-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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.
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
handling references for kibana_context and get_index_pattern expression functions #95224
Merged
ppisljar
merged 10 commits into
elastic:master
from
ppisljar:expressions/extract_inject
Mar 25, 2021
Merged
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
b1914da
kibana context extract/inject
ppisljar 0064999
extractin index pattern id
ppisljar 6c1fdbb
updating executor
ppisljar 362ade4
removing getSavedObject handler
ppisljar b7b86cb
updating docs
ppisljar 10ee5b7
Apply suggestions from code review
ppisljar f937922
Apply suggestions from code review
ppisljar e6afc6f
review feedback
ppisljar 7adcad1
review feedback
ppisljar 9fb63dc
Merge branch 'master' into expressions/extract_inject
ppisljar File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
13 changes: 0 additions & 13 deletions
13
...lic/kibana-plugin-plugins-expressions-public.executioncontext.getsavedobject.md
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
13 changes: 0 additions & 13 deletions
13
...ver/kibana-plugin-plugins-expressions-server.executioncontext.getsavedobject.md
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
39 changes: 39 additions & 0 deletions
39
src/plugins/data/public/search/expressions/kibana_context.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0 and the Server Side Public License, v 1; you may not use this file except | ||
* in compliance with, at your election, the Elastic License 2.0 or the Server | ||
* Side Public License, v 1. | ||
*/ | ||
|
||
import { StartServicesAccessor } from 'src/core/public'; | ||
import { getKibanaContextFn } from '../../../common/search/expressions'; | ||
import { DataPublicPluginStart, DataStartDependencies } from '../../types'; | ||
import { SavedObjectsClientCommon } from '../../../common/index_patterns'; | ||
|
||
/** | ||
* This is some glue code that takes in `core.getStartServices`, extracts the dependencies | ||
* needed for this function, and wraps them behind a `getStartDependencies` function that | ||
* is then called at runtime. | ||
* | ||
* We do this so that we can be explicit about exactly which dependencies the function | ||
* requires, without cluttering up the top-level `plugin.ts` with this logic. It also | ||
* makes testing the expression function a bit easier since `getStartDependencies` is | ||
* the only thing you should need to mock. | ||
* | ||
* @param getStartServices - core's StartServicesAccessor for this plugin | ||
* | ||
* @internal | ||
*/ | ||
export function getKibanaContext({ | ||
getStartServices, | ||
}: { | ||
getStartServices: StartServicesAccessor<DataStartDependencies, DataPublicPluginStart>; | ||
}) { | ||
return getKibanaContextFn(async () => { | ||
const [core] = await getStartServices(); | ||
return { | ||
savedObjectsClient: (core.savedObjects.client as unknown) as SavedObjectsClientCommon, | ||
}; | ||
}); | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
46 changes: 46 additions & 0 deletions
46
src/plugins/data/server/search/expressions/kibana_context.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0 and the Server Side Public License, v 1; you may not use this file except | ||
* in compliance with, at your election, the Elastic License 2.0 or the Server | ||
* Side Public License, v 1. | ||
*/ | ||
|
||
import { StartServicesAccessor } from 'src/core/server'; | ||
import { getKibanaContextFn } from '../../../common/search/expressions'; | ||
import { DataPluginStart, DataPluginStartDependencies } from '../../plugin'; | ||
import { SavedObjectsClientCommon } from '../../../common/index_patterns'; | ||
|
||
/** | ||
* This is some glue code that takes in `core.getStartServices`, extracts the dependencies | ||
* needed for this function, and wraps them behind a `getStartDependencies` function that | ||
* is then called at runtime. | ||
* | ||
* We do this so that we can be explicit about exactly which dependencies the function | ||
* requires, without cluttering up the top-level `plugin.ts` with this logic. It also | ||
* makes testing the expression function a bit easier since `getStartDependencies` is | ||
* the only thing you should need to mock. | ||
* | ||
* @param getStartServices - core's StartServicesAccessor for this plugin | ||
* | ||
* @internal | ||
*/ | ||
export function getKibanaContext({ | ||
getStartServices, | ||
}: { | ||
getStartServices: StartServicesAccessor<DataPluginStartDependencies, DataPluginStart>; | ||
}) { | ||
return getKibanaContextFn(async (getKibanaRequest) => { | ||
const request = getKibanaRequest && getKibanaRequest(); | ||
if (!request) { | ||
throw new Error('KIBANA_CONTEXT_KIBANA_REQUEST_MISSING'); | ||
} | ||
|
||
const [{ savedObjects }] = await getStartServices(); | ||
return { | ||
savedObjectsClient: (savedObjects.getScopedClient( | ||
request | ||
) as any) as SavedObjectsClientCommon, | ||
}; | ||
}); | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit confused about what does
getKibanaRequest
resolve to, asKibanaRequest
is the server-side type and makes sense only on the server.Not specific to this PR:
Also, I'm skeptical about using
KibanaRequest
type inexpressions
plugin. Maybe there is a way to create our own wrapper around it, call itExpressionUserContext
.The problem I see with it is that it assumes that you can authenticate only using
KibanaRequest
. Also, it exportsKibanaRequest
inExecutionContext
interface fromexpressions
plugin, making it our public API, but when Core will want to change something inKibanaRequest
, they might need to do changes toexpressions
plugin and it might be breaking change inexpressions
plugin. Just some thoughts.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getKibanaRequest on the client will be undefined, but the getStartServices function on the client doesn't expect it. this is just so we can keep the same code in common.
i agree the whole kibana request thing is a bit weird and we should probably do somethin about it in the future