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

Set query database to default if unspecified #863

Merged
merged 3 commits into from May 10, 2024

Conversation

aangelisc
Copy link
Contributor

If a query is missing the database property the current behaviour will cause that query to fail. This change will set the database property on the query to the data source default if available.

Fixes #615

- Set database from data source default if not specified in query
- Throw error if no database on query and no default in data source
@aangelisc aangelisc added effort/small changelog PRs to include in the CHANGELOG labels Apr 16, 2024
@aangelisc aangelisc self-assigned this Apr 16, 2024
@aangelisc aangelisc requested a review from a team as a code owner April 16, 2024 10:02
Copy link

github-actions bot commented Apr 16, 2024

Use the following command to run this PR with Docker at http://localhost:3000:

docker run --rm -p 3000:3000 grafana/plugin-builds:9116b23e435c16cf5808766e66a6bf847194b981pre

Copy link
Contributor

@bossinc bossinc left a comment

Choose a reason for hiding this comment

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

I am not sure if we should do this now that we can select a cluster. If the selected cluster doesn't have the default database (its in another cluster) that seems strange. What do you think?

@aangelisc
Copy link
Contributor Author

@bossinc that's a good call-out. The only reason I'm thinking this is worthwhile is to solve the original issue of not wanting to specify the database in generated dashboards. Maybe we should add a default cluster option too?

@bossinc
Copy link
Contributor

bossinc commented Apr 19, 2024

@bossinc that's a good call-out. The only reason I'm thinking this is worthwhile is to solve the original issue of not wanting to specify the database in generated dashboards. Maybe we should add a default cluster option too?

@aangelisc lets talk about this next week. I am mixed about how we are doing defaults

bossinc
bossinc previously approved these changes May 10, 2024
# Conflicts:
#	pkg/azuredx/datasource_test.go
@aangelisc aangelisc merged commit 3c3b579 into main May 10, 2024
8 checks passed
@aangelisc aangelisc deleted the andreas/set-default-database branch May 10, 2024 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog PRs to include in the CHANGELOG effort/small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for empty or missing database key in dashboard JSON, fallback to default
2 participants