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

[DISCUSS] Visualization saved object migration plan #29382

Closed
stacey-gammon opened this issue Jan 25, 2019 · 10 comments
Closed

[DISCUSS] Visualization saved object migration plan #29382

stacey-gammon opened this issue Jan 25, 2019 · 10 comments
Labels

Comments

@stacey-gammon
Copy link
Contributor

stacey-gammon commented Jan 25, 2019

The original plan was for #21645 to be introduced in a minor but that wasn't the intention of the migration API - to allow for big changes to objects to be introduced in a minor.

Let's talk about why that migration is required. In the issue above it's written:

With the introduction of the new pipeline (#19813) for visualizations we need to change the saved object format for visualizations.

but I think we already have now visualizations working with the pipeline in their old saved object format.

@ppisljar do you mind filling in some details more specifically on that migration plan? I could be wrong but I think it might be mainly for the New Visual Editor and so we don't have to support two ways of doing everything. Is that accurate?

I see some options being:

  1. Use a new visualization type that is created by the New Visual Editor.
    Cons: maintenance overhead and worse UX with them not being interchangeable.
  2. Add the expression field to the existing object, rather than replacing everything else with it.
    Cons: syncing two sources of truth.
  3. Have an in memory migration step - every time a visualization saved object is loaded, convert the contents to an expression. Use this in the code. Plan for migration in 8.0.
  4. Bite the bullet and do the change anyway in a minor.

Maybe 3. is a reasonable option? I don't really know, I've only thought about this for about 30 minutes, so I could be missing a lot of context.

Interested to hear others input.

@stacey-gammon stacey-gammon added discuss Team:Visualizations Visualization editors, elastic-charts and infrastructure :AppArch labels Jan 25, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app

@lukeelmers
Copy link
Member

@stacey-gammon Option 3 sounds reasonable to me too, especially since the code to do that in-memory migration would be reusable when it came time to do the "real" migration. Though I'd be interested in hearing @ppisljar's thoughts on all of this.

@ppisljar
Copy link
Member

currently, we build expression from the current saved object on-the-fly, everytime you render your visualization.. Pretty much option 3).

the plan was to do this conversion with the migration API, everytime you add/create a saved object.

the reaons why we want new saved object:

  • well, the current one is full of legacy things we no longer use
  • the current one is not flexible enough (we can't save canvas panel as it ....)
  1. ... you listed the cons, 2) i think this one doesn't make sense at all, 3) we do this now, 4) this is what i would go for

i still think this is not a breaking change ... nothing will break :) except downstream projects, that would be loading saved objects from kibana and expecting them to be of specific format ....
thaaats really not functionality we would support, we don't do bwc on API level or level of saved objects as far as i know. Saving objects to kibana for them will not be an issue, its only loading.

I think thats fine ... i mean plugin author anyway will need to change the plugin every minor.

@ppisljar
Copy link
Member

oh. and no, this has nothing to do with new editor, its purely about visualizations, what they were, what they are right now...

before they were.... just some json config pretty much, describing how to render and what to render
now we have an expression describing that, which is a looot more flexible

we can always convert from this json to expression ... but the other way around does not work
which means that currently our system allows you to create visualizations that can not be saved (as the saved object doesn't support all that can actually be done)

migration api could ALWAYS convert from json to expression ... no issues there.

@epixa
Copy link
Contributor

epixa commented Jan 26, 2019

Breaking a saved object api request would be a breaking change, so it should be avoided in minors. The saved object API is a feature of Kibana.

The incremental approach is an interesting one. I assume this transformation is being applied at read time in the UI and subsequently persisted when saved. Effectively that means that the existing saved object format is supported at the API level... to an extent. That’s an interesting gray area I hadn’t considered.

Can we prompt the user when they go to save a legacy object? Some sort of “saving this object will automatically ‘upgrade’ it behind the scenes to a new more powerful API format, confirm and remember this choice” modal?

@epixa
Copy link
Contributor

epixa commented Jan 26, 2019

And also, can someone clarify the details about why we can’t do the entire migration to the new format in 7.0?

@mattkime
Copy link
Contributor

Seems like we could use some documentation regarding what format / interface changes constitute breaking changes.

@rayafratkina
Copy link
Contributor

If there is a multi-user instance of Kibana, what happens when one of the users chooses "save in the new format" option? What level of privileges does the user need to save as new format?

Also, can we introduce the new format alongside the old one in a minor and say the old one is deprecated? This way old content continues to load and new content is saved in the new format. But you can't move objects from the new version of Kibana into the older versions (which I think is fine)

@ppisljar
Copy link
Member

why we can't do it in 7.0: well, we don't have time (or resources).

Breaking a saved object api request would be a breaking change, so it should be avoided in minors. The saved object API is a feature of Kibana.

well ... we are not breaking API request, api will still look the same

regarding multi users: this would be for whole kibana and for all users. We are talking about moving to new saved object format for visualizations, when we introduce that (in some version), that will be global, for everyone

@stacey-gammon
Copy link
Contributor Author

Closing as confirmed with Jim and Court this is acceptable to do in a minor

@timroes timroes removed Team:Visualizations Visualization editors, elastic-charts and infrastructure labels Mar 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

8 participants