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

Update dependencies #244

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Update dependencies #244

wants to merge 1 commit into from

Conversation

nineinchnick
Copy link
Member

This includes migrating to the new plugin toolkit and a major version update of Grafana.

@cla-bot cla-bot bot added the cla-signed label Aug 25, 2023
@@ -3,8 +3,8 @@ module github.com/trinodb/grafana-trino
go 1.18

require (
github.com/grafana/grafana-plugin-sdk-go v0.162.0
github.com/grafana/sqlds/v2 v2.5.1
github.com/grafana/grafana-plugin-sdk-go v0.173.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Note that this version is still vulnerable to https://grafana.com/security/security-advisories/cve-2024-8986/. To avoid that this dependency should be updated to at least v0.250.0.

Copy link
Member Author

Choose a reason for hiding this comment

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

This PR stalled, because I was waiting for a new secret used to sign the artifact during releases. The new secret is required after updating some of the tooling used here. It's already set in the repository, so I can get back to this now. I'll refresh all dependencies again.

Copy link
Contributor

Choose a reason for hiding this comment

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

Awesome, thanks 🙏

@robmoore-i
Copy link
Contributor

@nineinchnick Is there anything I can do to help get this merged soon?

@nineinchnick
Copy link
Member Author

@nineinchnick Is there anything I can do to help get this merged soon?

Sorry for the delay, but I can only work on this in my spare time. I can try getting it done this week, but I can't commit to any specific dates.

This update is significant, because it switches over to a new plugin SDK. I wanted to get this done before upgrading to a new Grafana major version. I see I left the CI green, so I only need to check if the release workflow will work, which signs the artifacts.

I'd then open another PR with more updates, this is where you could help - maybe you can create a new PR based on this branch.

@robmoore-i
Copy link
Contributor

@nineinchnick Is there anything I can do to help get this merged soon?

Sorry for the delay, but I can only work on this in my spare time. I can try getting it done this week, but I can't commit to any specific dates.

Ah sorry, my bad for making you feel rushed. I had thought looking after this plugin was part of your duties in Starburst.

This update is significant, because it switches over to a new plugin SDK. I wanted to get this done before upgrading to a new Grafana major version. I see I left the CI green, so I only need to check if the release workflow will work, which signs the artifacts.

Alright fab 👍 Sounds quite promising.

I'd then open another PR with more updates, this is where you could help - maybe you can create a new PR based on this branch.

Awesome thanks, yes I'll have a go at this on Friday.

@robmoore-i
Copy link
Contributor

I've had a go and created #269. It updates go.mod with updated dependencies, but also it updates go 1.18 to go 1.21, which matches the version used by the Grafana plugin SDK and also the version used in the GitHub action as updated by this branch. There were also a couple of minor code changes needed.

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

Successfully merging this pull request may close these issues.

2 participants