Skip to content

Fix top level field mappings issue, use dynamic: 'false' instead of dynamic: false#226988

Merged
gsoldevila merged 2 commits intoelastic:mainfrom
gsoldevila:use-string-for-dynamic-property
Jul 14, 2025
Merged

Fix top level field mappings issue, use dynamic: 'false' instead of dynamic: false#226988
gsoldevila merged 2 commits intoelastic:mainfrom
gsoldevila:use-string-for-dynamic-property

Conversation

@gsoldevila
Copy link
Copy Markdown
Member

@gsoldevila gsoldevila commented Jul 8, 2025 •
edited
Loading

Summary

Closes https://github.com/elastic/kibana-team/issues/1802

The PR aims at aligning the in-memory representation with the one coming from ES mappings, to prevent such discrepancies.

@gsoldevila gsoldevila requested review from a team as code owners July 8, 2025 11:39
@gsoldevila gsoldevila added Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting labels Jul 8, 2025
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Jul 8, 2025

Pinging @elastic/kibana-core (Team:Core)

@gsoldevila gsoldevila self-assigned this Jul 8, 2025
@gsoldevila gsoldevila changed the title Use string for dynamic property Use string for dynamic: property on index mappings Jul 8, 2025
rudolf
rudolf reviewed Jul 8, 2025
View reviewed changes
src/core/packages/saved-objects/server/src/mapping_definition.ts Outdated
* *never* use `dynamic: 'true'`.
*/
dynamic?: false | 'strict';
dynamic?: MappingDynamicMapping;
Copy link
Copy Markdown
Contributor

@rudolf rudolf Jul 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This type is defined as:
export type MappingDynamicMapping = boolean | 'strict' | 'runtime' | 'true' | 'false';

And we want to avoid anyone using boolean and 'true' not sure if 'runtime' would even work with saved objects.

So I would maybe rather type it as 'false' | 'strict'

Copy link
Copy Markdown
Member Author

@gsoldevila gsoldevila Jul 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a good point! Addressed with aa30c1b

@gsoldevila gsoldevila force-pushed the use-string-for-dynamic-property branch from 755b88b to 05711a7 Compare July 10, 2025 08:52
@gsoldevila gsoldevila requested review from a team as code owners July 10, 2025 15:26
@gsoldevila gsoldevila requested a review from a team July 10, 2025 15:26
@gsoldevila gsoldevila requested review from a team as code owners July 10, 2025 15:26
@gsoldevila gsoldevila force-pushed the use-string-for-dynamic-property branch from ed69995 to 32d4b40 Compare July 11, 2025 08:11
@gsoldevila gsoldevila requested a review from rudolf July 11, 2025 08:43
@gsoldevila gsoldevila force-pushed the use-string-for-dynamic-property branch from 32d4b40 to c15e88e Compare July 11, 2025 14:55
@gsoldevila gsoldevila removed request for a team and xcrzx July 11, 2025 14:56
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Jul 14, 2025

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
@kbn/core-saved-objects-server 136 138 +2

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
@kbn/core 888 890 +2
Unknown metric groups

API count

id before after diff
@kbn/core-saved-objects-server 572 574 +2

History

cc @gsoldevila

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@janmonschke janmonschke janmonschke approved these changes

@rudolf rudolf rudolf approved these changes

@SiddharthMantri SiddharthMantri SiddharthMantri approved these changes

@ThomThomson ThomThomson ThomThomson approved these changes

@juliaElastic juliaElastic juliaElastic approved these changes

Assignees

@gsoldevila gsoldevila

Labels

backport:skip This PR does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:actionable-obs Formerly "obs-ux-management", responsible for SLO, o11y alerting, significant events, & synthetics. Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// Team:Fleet Team label for Observability Data Collection Fleet team Team:obs-ux-infra_services - DEPRECATED DEPRECATED - Use Team:obs-presentation. v9.2.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

10 participants

@gsoldevila @elasticmachine @janmonschke @rudolf @SiddharthMantri @ThomThomson @juliaElastic @smith @rmyz @kibanamachine