-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
By value migration script #129303
Comments
Pinging @elastic/kibana-vis-editors @elastic/kibana-vis-editors-external (Team:VisEditors) |
cc @ruflin |
@elastic/ecosystem Lets collaborate with the team from @flash1293 on this script. @joshdover You are likely interested in this too as it will speed up the installation of packages. |
The https://github.com/flash1293/legacy_vis_analyzer script is already doing the easy part of this (finding dashboards and visualizations and associating them by id) |
Hey Joe, FYI We use
|
@mtojek Thanks for reaching out.
The old format is still supported, this is just an addition (and it's compatible with all tooling). Using the inlined variant should be the default choice though and there should be a specific reason if separate saved objects are used.
No, there are still good use cases to support this (e.g. one-off visualizations not part of a dashboard or visualizations which should always stay in sync across multiple dashboards, even after being edited by the user).
That's not necessary, everything will stay compatible. There are already some dashboards which are fully by value: https://github.com/elastic/integrations/tree/main/packages/ti_abusech/kibana |
@ruflin @joshdover I don't know what was the reason to separate Kibana objects into different directories. Maybe we should iterate on this? |
By the time the package-spec was created, by value did not exist yet. So this was the only option. I think moving forward, the majority of our dashboards with the visulisations inside should be just in the |
It's less about the directory structure within the integrations repo and more about how it translates into saved objects within Kibana. The current approach of having a single saved object per visualization has some downsides:
The reason the structure with separate visualization saved objects is used because a lot of these dashboards (or at least the first version of them) predate the "by value" (inlining of visualizations into the dashboard) feature. |
One small note to add - saved searches can't be done "by value" yet, so they will definitely stay, at least in the mid-term. map, lens and visualization saved objects can be inlined. |
Another good point by @joshdover about why inlining is beneficial: On using the global search bar, it's harder than necessary at the moment to find the dashboard because there will always be lots of visualizations matching the same search. |
It means that in most cases it would be recommended to switch to the inline form dashboards objects. That's what I expected and hence was my follow-up question regarding adjustments in elastic-package. If we decide to migrate all dashboards and don't update the I guess that we can evaluate which object types can be included in the dashboards based on the spec definitions. |
@mtojek It would be nice to introduce some kind of linting rule to prevent accidentally adding by reference visualizations in the future (without completely preventing it). However, I'm not sure how common it will be - if an integration author is going to put together a dashboard for a new integration by going to Kibana and configuring it through the UI, they will end up with inlined by value visualizations not with separate saved objects, because this is what's happening by default when adding panels to a dashboard. I have set up a tracking dashboard for visualize->lens and by value migration: IMHO we can keep an eye on that for now and introduce more tooling in case it becomes an issue. |
@ruflin @cmacknz I put together a first version of the vis inliner here: https://github.com/flash1293/legacy_vis_analyzer (do not look at the code, it's horrible). Could you give it a test run to see whether it works? |
@lalit-satapathy Pulling you in here as this is also interesting for your team. @cmacknz I assume based on your 👍 you'll give it a try. |
I opened an issue in package-spec elastic/package-spec#316 to help with migration to "by value" by notifying engineers about it. |
Yes, I'll give the migration a try. I have upcoming PTO though so it might not happen for 2 weeks or so but it is on my list of things to do. If we want to try it sooner we'll have to find somebody else to try it out. |
@mtojek Do you have some time to give https://github.com/flash1293/legacy_vis_analyzer a try? Always happy to hop on a zoom to talk through it. |
I created an issue for this: elastic/elastic-package#791 |
Script is available, if there are problems/bugs we'll open a new issue |
Right now, a lot of integrations dashboards are still built "by value". This means there are separate saved objects for each visualization. To ease migration to "by value" dashboards which don't have this issue, a helper script should be provided that performs the following tasks given a folder:
This allows maintainers to run the script, check whether the dashboards still work as expected and create a PR.
The text was updated successfully, but these errors were encountered: