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.

Sign up for GitHub

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

Jump to bottom

Chore: small improvements #37

Merged
merged 1 commit into from
Nov 11, 2021
Merged

Chore: small improvements #37

merged 1 commit into from
Nov 11, 2021

Conversation

andresmgot
Copy link
Contributor

  • SQL Datasource: Separate configuration storing from instance initialization.
  • SQL Routes: Allow to send any resource, not only []string

@andresmgot andresmgot requested review from a team, iwysiu and vickyyyyyyy and removed request for a team November 10, 2021 10:03
iwysiu
iwysiu approved these changes Nov 10, 2021
View reviewed changes
Copy link
Contributor

@iwysiu iwysiu 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 what's the benefit from separating the initialization and the config? Do we ever want an unconfigured datasource?

@andresmgot
Copy link
Contributor Author

andresmgot commented Nov 11, 2021 •
edited

LGTM! But what's the benefit from separating the initialization and the config? Do we ever want an unconfigured datasource?

Good question, this was a actually a bug. The instance should contain multiple configurations (one per datasource, the instance is shared) but the only way to add one was the initialization function (so it only held one).

@andresmgot andresmgot merged commit 50189f1 into main Nov 11, 2021
@andresmgot andresmgot deleted the storeConfig branch November 11, 2021 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iwysiu iwysiu iwysiu approved these changes

@vickyyyyyyy vickyyyyyyy Awaiting requested review from vickyyyyyyy vickyyyyyyy is a code owner automatically assigned from grafana/aws-datasources

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@andresmgot @iwysiu