-
Notifications
You must be signed in to change notification settings - Fork 7
chore: refactor dev image tag name resolver #118
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
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.
Pull Request Overview
This PR refactors the dev image tag name resolver by simplifying the logic from a complex tag matching and fallback algorithm to a straightforward "latest tag" approach.
- Replaces the complex npm-based tag resolution with a simpler DockerHub API approach
- Removes fallback logic for finding "next best" tags when exact matches aren't found
- Improves code organization with better documentation and cleaner structure
Reviewed Changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| e2e-version/npm-to-docker-image.js | Completely removed - contained complex tag matching and fallback logic |
| e2e-version/index.js | Updated import to use new get-dev-image-tag module |
| e2e-version/get-dev-image-tag.js | New implementation that fetches latest Grafana dev tag from DockerHub API |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
hugohaggmark
left a comment
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.
I don't know the complete history here but wasn't the reason to match npm packages with docker image to make sure we only test on changes to published npm packages?
But I don't have any strong opinion about this and this PR looks great 🎉 but I'll leave the approval to someone on the team with more history 👍
The reason the e2e-versions Action includes the most recent grafana-dev image is to verify that the plugin is forwards-compatible with Grafana. No grafana npm deps are installed upon running Playwright tests, so there's no need for the grafana npm packages to have a corresponding npm release for the certain build number that is currently tested. |
tolzhabayev
left a comment
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.
🚀 awesome
Previously, the e2e version resolver determined the Grafana canary dev image tag by first checking NPM for the latest build number, and then trying to match it with a corresponding tag from the Docker Hub API. This workflow has proved unreliable lately: in some cases, NPM published a new version before the corresponding image appeared on Docker Hub. As a result, the lookup failed because no matching build could be found.
This PR removes the dependency on NPM and instead retrieves the tag directly from the Docker Hub API (
https://registry.hub.docker.com/v2/repositories/grafana/grafana-dev/tags). The API returns tags in reverse chronological order of creation (last updated). While it’s generally safe to assume the latest tag is also the highest build number, to be certain the logic now scans the 25 most recent tags and selects the one with the highest build number.Timeouts and retries for Docker Hub API calls have also been increased to reduce the risk of intermittent network-related failures observed recently.
This change has been tested here.