-
Notifications
You must be signed in to change notification settings - Fork 15
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
Update to sqlutil helpers #115
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems reasonable to me! I long for the day when grafana-aws-sdk doesn't know what sqlds or sqlutils are, but today is not that day, and in the meantime it makes sense to me that we'd want it all referencing one place!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Also thanks for handling the v3 context update
pkg/sql/api/api.go
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It may also make sense to add aws.Context
to the arguments for the SQL.Stop
function, since it's also a request.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, but I think I'll leave that for a separate PR.
go.mod
Outdated
@@ -4,18 +4,22 @@ go 1.21 | |||
|
|||
toolchain go1.21.3 | |||
|
|||
// TODO: remove both of the following before release |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reminder to remove these when you merge
This updates the sdk to bring it in sync with changes to
sqlds
andgrafana-plugin-sdk-go
related to grafana/sqlds#85 and grafana/grafana-plugin-sdk-go#858.Depends on grafana/sqlds#105, grafana/grafana-plugin-sdk-go#862 and grafana/grafana-plugin-sdk-go#863.