Dataplane: Deprecate timeseries-many in favor of timeseries-multi #59070
Conversation
bohandley
added
add to changelog
area/dataframe
no-backport
bohandley
requested review from
joshhunt,
JoaoSilvaGrafana,
mckn,
oscarkilhed and
supilee
and removed request for
a team
| @@ -7,7 +7,7 @@ | |||
| export enum DataFrameType { | |||
| TimeSeriesWide = 'timeseries-wide', | |||
| TimeSeriesLong = 'timeseries-long', | |||
| TimeSeriesMany = 'timeseries-many', | |||
There was a problem hiding this comment.
Is this a breaking change in our public (typescript) API?
There was a problem hiding this comment.
This is a good question! @kylebrandt @ryantxu Is this supposed to be a breaking change or not? We need to consider the typescript API as Josh mentioned.
There was a problem hiding this comment.
what is and isn't a breaking changes for plugins is tricky to define for the frontend. Typescript is not a runtime concern, so this will not break any existing plugins from running. But it could prevent them from being built.
From the outside, a simple way out would be to actually deprecate (rather than remove) the enum value with @deprecated jsdoc annotation, and then remove in grafana 10.
There was a problem hiding this comment.
There was a problem hiding this comment.
Yup deprecating not removing
There was a problem hiding this comment.
Can we add something to point to the replacement, e.g. "use TimeSeriesMulti instead", or "@deprecated in favor of TimeSeriesMulti"
There was a problem hiding this comment.
I updated the text @kylebrandt
|
Drone build failed: https://drone.grafana.net/grafana/grafana-enterprise/42736 |
grafanabot
added
the
levitate breaking change
grafanabot
removed
the
levitate breaking change
|
Drone build failed: https://drone.grafana.net/grafana/grafana-enterprise/42741 |
|
This is fine... but we could/should likely update alll the frontend references to use multi I think this: https://github.com/grafana/grafana/blob/main/public/app/features/transformers/prepareTimeSeries/prepareTimeSeries.ts#L294 is the only line that would need to check both values (many or mullti) |
Thank you Ryan! This is just the enumerable for selecting the format, right? |
What is this feature?
This work is to deprecate the dataframe type timeseries-many for timeseries-multi.
Why do we need this feature?
This work is needed for grafana/grafana-plugin-sdk-go#560 so that the frontend works with the new type.
Who is this feature for?
This feature is for the dataplane project.
Special notes for your reviewer:
This is a first try at deprecating timeseries-many, so any suggestions to get this working are appreciated!