Skip to content
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

[data view field editor] Allow editing of DataViewLazy #186348

Merged
merged 10 commits into from
Jun 22, 2024

Conversation

mattkime
Copy link
Contributor

@mattkime mattkime commented Jun 18, 2024

Summary

Data view field editor will now allow editing of fields when provided with a DataViewLazy object. Previously it required a DataView object. This change makes it easier for API consumers to move from DataView to DataViewLazy usage.

Internally the data view field editor still uses DataView objects since some of the validation code expects a complete field list. The validation code would need to be rewritten to assume incompete field lists. There is the potential for a performance hit when loading a large field list. After the initial load it will be loaded from the browser cache which should be performant.

Part of #178926

@mattkime mattkime self-assigned this Jun 18, 2024
@mattkime mattkime changed the title field editor can use dataView or dataViewLazy [data view field editor] Allow editing of DataViewLazy Jun 19, 2024
@mattkime mattkime added Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. Feature:Data Views Data Views code and UI - index patterns before 8.0 Feature:Kibana Management Feature label for Data Views, Advanced Setting, Saved Object management pages release_note:skip Skip the PR/issue when compiling release notes labels Jun 19, 2024
@mattkime mattkime marked this pull request as ready for review June 19, 2024 18:55
@mattkime mattkime requested review from a team as code owners June 19, 2024 18:55
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

Copy link
Member

@jgowdyelastic jgowdyelastic left a comment

Choose a reason for hiding this comment

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

ML changes LGTM

Copy link
Member

@lukasolson lukasolson left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@michaelolo24 michaelolo24 left a comment

Choose a reason for hiding this comment

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

LGTM!

@mattkime
Copy link
Contributor Author

/ci

@mattkime mattkime enabled auto-merge (squash) June 22, 2024 03:52
@mattkime
Copy link
Contributor Author

/ci

@mattkime mattkime requested review from a team as code owners June 22, 2024 05:39
@botelastic botelastic bot added the ci:project-deploy-observability Create an Observability project label Jun 22, 2024
@mattkime
Copy link
Contributor Author

/ci

@mattkime mattkime force-pushed the data_view_mgmt_use_data_view_lazy branch from d677845 to 9f4c569 Compare June 22, 2024 05:49
@mattkime
Copy link
Contributor Author

/ci

@mattkime mattkime removed request for a team June 22, 2024 05:50
@mattkime
Copy link
Contributor Author

/ci

@kibana-ci
Copy link
Collaborator

kibana-ci commented Jun 22, 2024

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
cloudSecurityPosture 450.0KB 450.0KB +11.0B
dataViewFieldEditor 176.3KB 176.3KB +1.0B
dataViewManagement 137.0KB 137.0KB +12.0B
dataVisualizer 759.8KB 759.8KB +34.0B
discover 808.7KB 808.7KB +29.0B
esqlDataGrid 114.3KB 114.3KB +11.0B
lens 1.5MB 1.5MB +12.0B
securitySolution 13.6MB 13.6MB +96.0B
slo 867.5KB 867.6KB +23.0B
total +229.0B

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
dataViewFieldEditor 0 1 +1

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
dataViewFieldEditor 26.3KB 26.5KB +249.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @mattkime

@mattkime mattkime removed the ci:project-deploy-observability Create an Observability project label Jun 22, 2024
@mattkime mattkime merged commit 74a202a into elastic:main Jun 22, 2024
51 of 52 checks passed
@kibanamachine kibanamachine added v8.15.0 backport:skip This commit does not require backporting labels Jun 22, 2024
bhapas pushed a commit to bhapas/kibana that referenced this pull request Jun 24, 2024
## Summary

Data view field editor will now allow editing of fields when provided
with a DataViewLazy object. Previously it required a DataView object.
This change makes it easier for API consumers to move from DataView to
DataViewLazy usage.

Internally the data view field editor still uses DataView objects since
some of the validation code expects a complete field list. The
validation code would need to be rewritten to assume incompete field
lists. There is the potential for a performance hit when loading a large
field list. After the initial load it will be loaded from the browser
cache which should be performant.

Part of elastic#178926
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Data Views Data Views code and UI - index patterns before 8.0 Feature:Kibana Management Feature label for Data Views, Advanced Setting, Saved Object management pages release_note:skip Skip the PR/issue when compiling release notes Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. v8.15.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants