-
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
[Mappings Editor] Disable _source field in serverless #181712
[Mappings Editor] Disable _source field in serverless #181712
Conversation
/ci |
1 similar comment
/ci |
76ec03e
to
e019e0a
Compare
@@ -52,6 +52,11 @@ const schemaLatest = schema.object( | |||
// We take this approach in order to have a central place (serverless.yml) for serverless config across Kibana | |||
serverless: schema.boolean({ defaultValue: true }), | |||
}), | |||
enableMappingsSourceField: offeringBasedSchema({ |
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 was thinking about renaming the config to enableMappingsSourceFieldConfiguration
but this might be too long. I'm open to any suggestions for the name of the config.
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.
What about enableMappingsSourceFieldConfig
or enableMappingsSourceFieldSection
? A little shorter, but not much 😅 . I'm also OK to leave it how you have it.
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.
Thanks for the suggestions! I like enableMappingsSourceFieldSection
since the component for the _source field configuration is called SourceFieldSection
. Added this change with 813297e.
b178de6
to
04dacec
Compare
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.
Core changes LGTM, did not test locally
Pinging @elastic/kibana-management (Team:Kibana Management) |
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.
Kibana security changes LGTM, just one nit request.
@@ -276,6 +276,7 @@ export default function ({ getService }: PluginFunctionalProviderContext) { | |||
'xpack.index_management.enableIndexStats (any)', | |||
'xpack.index_management.editableIndexSettings (any)', | |||
'xpack.index_management.enableDataStreamsStorageColumn (any)', | |||
'xpack.index_management.enableMappingsSourceField (any)', |
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.
Nit: could we just move this down below the comment with the other conditional flags, or comment as 'any' due to offering based schema?
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.
Sure, I moved it below the comment as well as the rest of the index management offering-based configs for consistency.
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.
Thanks for working on this! Tested on serverless ES and stateful and _source configuration was hidden/shown as expected.
One thing I noticed -
"Synthetic" mode is still supported via the API, but we haven't build the UI yet (follow up: #181609). If I create a template via the API with this configuration, the mappings editor doesn't handle it gracefully. Can you look into how difficult it would be to pass through the value?
Example request:
PUT _index_template/my_template
{
"index_patterns": ["foo*"],
"template": {
"mappings": {
"_source": {
"mode": "synthetic"
},
"properties": {
"host_name": {
"type": "keyword"
},
"created_at": {
"type": "date",
"format": "EEE MMM dd HH:mm:ss Z yyyy"
}
}
}
}
}
Result in the UI:
@@ -52,6 +52,11 @@ const schemaLatest = schema.object( | |||
// We take this approach in order to have a central place (serverless.yml) for serverless config across Kibana | |||
serverless: schema.boolean({ defaultValue: true }), | |||
}), | |||
enableMappingsSourceField: offeringBasedSchema({ |
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.
What about enableMappingsSourceFieldConfig
or enableMappingsSourceFieldSection
? A little shorter, but not much 😅 . I'm also OK to leave it how you have it.
Thanks for the review @alisonelizabeth!
That's a good call, thanks for bringing it up! I added the |
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.
Thanks @ElenaStoeva!
I noticed one more thing, but it may be related to an existing issue: #106006. If I create a template via the API with the synthetic mode, then click on the "Advanced" tab, then go to save, the mode is dropped from the request. Can you take a look and see how feasible it would be to prevent the field from getting dropped?
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.
Kibana security changes LGTM
Nice catch! I looked into this but couldn't find an easy fix - it seems the form overwrites the request with the current values in the form, and since there is no form data for the I can still investigate some more tomorrow to check again if there is any way to fix this without adding a field, I could have missed something. |
Hi @alisonelizabeth, I found that the
Therefore, I think it wouldn't make sense to display any of the existing source fields in the UI if the As a follow-up work, we can improve the UI to have a toggle between the two options (setting a mode or setting the existing fields). |
1ac4492
to
5d775db
Compare
💚 Build Succeeded
Metrics [docs]Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: cc @ElenaStoeva |
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.
Thanks @ElenaStoeva! Latest lgtm. Great work 🎉
* master: (1654 commits) Bump ejs from 3.1.9 to 3.1.10 Don't render exceptions flyout if data is loading (elastic#181588) Enable value list modal (elastic#181593) skip flaky suite (elastic#181777) skip failing test suite (elastic#182263) [Mappings Editor] Disable _source field in serverless (elastic#181712) [data.search] Fix unhandled promise rejections (elastic#181785) [Fleet] Fix logic for detecting first time Elastic Agent users (elastic#182214) [ML] Decouple data_visualizer from MapEmbeddable (elastic#181928) [ES|QL] Sorting accepts expressions (elastic#181916) [ML] Single Metric Viewer: ensures chart displays correctly when opening from a job annotation (elastic#182176) Adding optional Description field to Roles APIs (elastic#182039) Upgrade Markdown-it to 14.1.0 (elastic#182244) Bump xml-crypto from 5.0.0 to 6.0.0 [DOCS] Fix docs and screenshots for rule creation changes (elastic#181925) Update dependency elastic-apm-node to ^4.5.3 (main) (elastic#182236) [Obs AI Assistant] register alert details context in observability plugin (elastic#181501) Add `@typescript-eslint/no-floating-promises` (elastic#181456) [Playground] Propagate Error message into FE (elastic#182201) [ES|QL] Rename the setting to a more generic one and move to the general section (elastic#182074) ...
Closes #181555
Summary
This PR disables the
_source
field in the Mappings editor's advanced options when in serverless or when the_source.mode
property is set.How to test:
_source
field is not displayed and that creating a template doesn't add this property to the Es request._source
field still exists and works as expected._source.mode
property set (example Es request below), the whole _source field section is hidden.