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

Datasource settings: Add deprecation notice for database field #58647

Merged
merged 4 commits into from
Dec 2, 2022

Conversation

zoltanbedi
Copy link
Member

This PR adds deprecation notice for the database field to let others know that we are migrating away from that field and instead should use the JSONData field for database related settings.

This is also based on our conversation with @marefr in slack. https://raintank-corp.slack.com/archives/C01C4K8DETW/p1668075734953779

Related to grafana/grafana-plugin-sdk-go#555

@zoltanbedi zoltanbedi added this to the 9.3.0 milestone Nov 11, 2022
@zoltanbedi zoltanbedi requested a review from a team as a code owner November 11, 2022 13:10
@zoltanbedi zoltanbedi self-assigned this Nov 11, 2022
@zoltanbedi zoltanbedi requested a review from a team as a code owner November 11, 2022 13:10
@zoltanbedi zoltanbedi requested a review from a team November 11, 2022 13:10
@zoltanbedi zoltanbedi requested a review from a team as a code owner November 11, 2022 13:10
@zoltanbedi zoltanbedi requested review from joshhunt, ashharrison90, leventebalogh, mdvictor, supilee, wbrowne and marefr and removed request for a team November 11, 2022 13:10
@grafanabot
Copy link
Contributor

@@ -558,6 +558,10 @@ export interface DataSourceSettings<T extends DataSourceJsonData = DataSourceJso
access: string;
url: string;
user: string;
/**
* @deprecated -- use jsonData to store information related to database.
* This field should only be used by Elasticsearch and Influxdb.
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry if it was asked somewhere else already, but are we planning to remove the use of this from Elasticsearch and Influxdb as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry but I don't know anything about them. Maybe @grafana/observability-logs can answer?

Copy link
Contributor

Choose a reason for hiding this comment

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

we'd like to remove the use of this from Elasticsearch, but there's nothing planned yet (i created a github-issue to track this at #59098)

Copy link
Contributor

Choose a reason for hiding this comment

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

regarding Influxdb, pinging @grafana/observability-metrics 😄

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know of any plan to deprecate this, but it doesn't look like a huge lift to follow the plan laid out in the issue Gabor created above. I created an issue here #59115, so we don't loose track of this request.

@leandro-deveikis leandro-deveikis modified the milestones: 9.3.0, 9.3.0-beta1 Nov 14, 2022
@grafanabot grafanabot removed this from the 9.3.0-beta1 milestone Nov 15, 2022
@grafanabot
Copy link
Contributor

This pull request was removed from the 9.3.0-beta1 milestone because 9.3.0-beta1 is currently being released.

@zoltanbedi zoltanbedi added this to the 9.3.0 milestone Nov 22, 2022
@grafanabot
Copy link
Contributor

Copy link
Member

@marefr marefr left a comment

Choose a reason for hiding this comment

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

LGTM, but think @grafana/plugins-platform-frontend should approve this

@grafanabot grafanabot removed this from the 9.3.0 milestone Nov 30, 2022
@grafanabot
Copy link
Contributor

This pull request was removed from the 9.3.0 milestone because 9.3.0 is currently being released.

@zoltanbedi zoltanbedi added this to the 9.4.0 milestone Dec 2, 2022
Copy link
Contributor

@leventebalogh leventebalogh left a comment

Choose a reason for hiding this comment

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

🚢🇮🇹

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

Successfully merging this pull request may close these issues.

None yet

8 participants