You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by "PDGGK (via GitHub)" <gi...@apache.org> on 2026/07/29 10:57:16 UTC
[PR] Migrate the Grafana plugin build from grafana-toolkit to create-plugin [iotdb-extras]
PDGGK opened a new pull request, #118:
URL: https://github.com/apache/iotdb-extras/pull/118
## Why
`@grafana/toolkit` was archived by Grafana and no longer receives updates, so `npm run build` was pinned to an unmaintained toolchain. This migrates the plugin's frontend build to `@grafana/create-plugin`, which is the supported replacement.
## What changed
- **Build toolchain.** `grafana-toolkit plugin:build/test/dev` are replaced by webpack, SWC/Jest, the flat ESLint config and Playwright, all driven from the generated `.config/` directory. `.config/` is checked in as `create-plugin` intends, carries its own "do not edit" notice, and can be refreshed later with `npx @grafana/create-plugin update`.
- **Lockfile.** `yarn.lock` is removed in favour of `package-lock.json`, matching the npm-based scripts the new toolchain generates.
- **`grafanaDependency` moves from `>=9.3.0` to `>=12.3.0`.** This is the one compatibility decision in the change, so to be explicit about it: the plugin builds and is tested against `@grafana/data`, `@grafana/ui` and `@grafana/runtime` **13.1.0**. Those packages are webpack *externals* (`.config/bundler/externals.ts`), resolved from the host Grafana at runtime rather than bundled — so an API that exists in the 13.1.0 types but not on an older host does not fail the build, it fails at runtime in the user's Grafana with no bundled fallback. Declaring `>=12.3.0`, the floor `create-plugin` declares for this SDK generation, states the range we can actually stand behind. If keeping a lower floor matters more, that is worth saying now — it would mean pinning the `@grafana/*` packages down instead.
- **Two ESLint errors fixed.** The stricter config flags `react-hooks/immutability` in the tree-model query editors, where an array is mutated in place and the *same* reference is then passed to `onChange` — so a consumer comparing by reference sees no change. Both now pass a new array instead.
To be plain about what this leaves behind: `FromValue.tsx` and `SelectValue.tsx` still contain the identical append (`prefixPath[prefixPath.length] = ''; onChange(prefixPath)`) a few lines below the corrected one, where the rule does not flag it, plus two `splice`-based removals with the same shape. I left them because these components have no test coverage and a toolchain migration is the wrong place to change untested UI behaviour — but I am happy to fix the remaining sites in a separate change with tests.
- **CI workflow updated in the same change.** `.github/workflows/grafana-plugin.yml` triggers on `connectors/grafana-plugin/**`, pins `node-version: "14.x"`, installs with `yarn install --frozen-lockfile` and keys both caches on `hashFiles('**/yarn.lock')`. Since this change removes `yarn.lock`, that workflow would fail at install, and Node 14 cannot run the new toolchain in any case. It now takes the Node version from the plugin's `.nvmrc` — so it cannot drift away from the toolchain again — uses `setup-node`'s npm cache keyed on `package-lock.json`, and runs `npm ci && npm run build`. The Go/backend step is unchanged.
- **Development environment.** `docker-compose.yaml` now also starts a standalone IoTDB next to Grafana, and `provisioning/` wires the datasource to it (REST on 18080 for the tree-model modes, native Thrift on 6667 for the table-model mode). `npm run server` therefore brings up a Grafana with the plugin loaded *and* a server to query, instead of an empty instance.
One known rough edge in the generated config: `.config/supervisord/supervisord.conf` waits for a backend binary matching `gpx_iotdb*`, derived from the plugin *name*, while `plugin.json` declares `executable: gpx_apache_iotdb_datasource`. That only affects `DEVELOPMENT=true` (the delve path); with the default `DEVELOPMENT=false` the entrypoint runs Grafana directly, which is the path verified below. I have not hand-edited it, since `.config/` is generated and regenerated by `create-plugin update`.
## Verification
Toolchain: `typecheck`, `lint` (0 errors; 9 remaining warnings are pre-existing deprecation notices), `test:ci` (13 tests) and `build` all pass. The exact command chain the updated workflow runs — `npm ci && npm run build` — was run from a clean install to confirm `package-lock.json` and `package.json` are in sync, since `npm ci` fails hard if they are not.
End to end against real servers, not just a green build: the plugin was loaded into **Grafana 13.1.0** from `docker compose up`, with a real `apache/iotdb:2.0.8` behind it. Grafana registers the plugin (`Plugin registered pluginId=apache-iotdb-datasource`), the provisioned datasource connects, and a **table-model** query
```sql
SELECT time, device_id, temperature FROM sensors WHERE $__timeFilter(time) ORDER BY time
```
renders in a panel with the row-major result correctly pivoted into one series per tag value, and the same data shown in the table view.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Migrate the Grafana plugin build from grafana-toolkit to create-plugin [iotdb-extras]
Posted by "PDGGK (via GitHub)" <gi...@apache.org>.
PDGGK commented on PR #118:
URL: https://github.com/apache/iotdb-extras/pull/118#issuecomment-5180368128
Implemented all three outcomes from the list thread, plus the licence point. Pushed as `8409aaa`.
**1. Minimum version.** `grafanaDependency` stays at `>=12.3.0`, and the README now states the floor, why the externals model makes an untested floor meaningless, and how CI holds it. `CHANGELOG.md` records the compatibility break.
The smoke test is `minimum-grafana-version` in `grafana-plugin.yml`. It parses the floor out of `plugin.json` rather than hardcoding a version, boots that Grafana release alongside IoTDB, and asserts two different things: that the plugin's own config fields render — which is where an unresolved `@grafana/*` external actually surfaces — and that a table-model query returns rows through the backend and the native client. So raising or lowering the declared floor changes what is tested, with nothing else to keep in step. If the version cannot be parsed the step exits non-zero rather than falling back to a default, so it cannot silently drift into testing the wrong release.
I checked it fails for the right reasons rather than only that it passes: with Grafana stopped it fails; with IoTDB stopped only the query assertion fails while the two frontend ones stay green; with `dist/` removed it fails. The middle one is the case I most wanted to rule out, since a two-part smoke test is usually undone by one half masking the other.
**2. AGENT files.** Removed, and `.config/AGENTS/` added to `.gitignore` so a scaffold regeneration cannot bring them back.
**3. LICENCE / RAT.** The blanket `**/grafana-plugin/**` exclusion is replaced by a targeted list. Rather than guess at it I lifted the exclusion and measured: 68,238 unapproved files, all of them `node_modules/` and the downloaded `node/`. Excluding generated output, binaries and files that physically cannot carry a header brought that to 8, and adding the standard header to those eight hand-written files brought it to 0. `.config/` stays excluded with the reason inline — the scaffold regenerates it and marks it do-not-edit, so a header added there is lost on the next run.
One thing worth flagging rather than deciding on my own:
**On the Maven path.** Confirmed: before this PR neither Grafana profile was invoked by any CI job or by the Jenkinsfile, so nothing was building this module through Maven. The new job runs `-Pwith-grafana-plugin`, so that path is now exercised on every change to the plugin directory.
One thing I found while checking, since it bears on whether to go further: the module is reachable from two opt-in profiles, `with-grafana-plugin` and `with-grafana` — and `with-grafana` is part of the build line the top-level README recommends (`-Pwith-all-connectors,with-examples,with-springboot,with-grafana`), so anyone following the README is already building it locally. It is still in neither the default build nor `with-all-connectors`. If you would like it in a default profile as well I am happy to do that, but it changes build time for every connector, so it seemed worth asking rather than folding into this PR.
One observation I am deliberately keeping out of this PR since it is pre-existing and untouched here: `backend-compile.sh` runs `go get -u` and `go mod tidy` at build time, so a build can rewrite `go.mod`. Both of my builds bumped `magefile/mage` from v1.15.0 to v1.17.2; I reverted it before committing each time. I will open that separately once this lands.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Migrate the Grafana plugin build from grafana-toolkit to create-plugin [iotdb-extras]
Posted by "CritasWang (via GitHub)" <gi...@apache.org>.
CritasWang commented on PR #118:
URL: https://github.com/apache/iotdb-extras/pull/118#issuecomment-5161630997
Thanks for taking this on — moving off the archived `@grafana/toolkit` is the right direction, and the PR description is unusually thorough about its own trade-offs (the externals/`grafanaDependency` reasoning and the self-disclosed unfixed mutation sites are exactly the kind of thing that makes a migration reviewable).
Three things below. The first is blocking; the second is a suggestion that would have caught the first automatically; the third needs a call from the PMC rather than from you or me.
---
## 1. Blocking: `connectors/grafana-plugin/pom.xml` needs to change in this same PR
The PR description explains why `.github/workflows/grafana-plugin.yml` had to be updated here — "it is what the migration breaks". That reasoning is correct, but it applies equally to a second build path that this PR leaves untouched.
`connectors/grafana-plugin/pom.xml` drives the frontend build through `frontend-maven-plugin`, pinned to yarn:
```xml
<goal>install-node-and-yarn</goal>
<nodeVersion>v16.13.1</nodeVersion>
<yarnVersion>v1.22.17</yarnVersion>
...
<goal>yarn</goal> <arguments>install</arguments>
<goal>yarn</goal> <arguments>build</arguments>
```
After this PR that invocation runs **Node 16 + Yarn 1.22** against a project that:
- no longer has a `yarn.lock` (removed here),
- declares `"engines": { "node": ">=22" }` and ships `.nvmrc` = `22`,
- builds via `webpack -c ./.config/webpack/webpack.config.ts`, which needs `ts-node` / `@swc/core` from the new toolchain.
So `mvn -Pwith-grafana-plugin package` — which `CLAUDE.md` documents as the canonical entrypoint for this plugin, with an explicit "do not invoke npm directly unless you know what the Maven wrapper is doing" — breaks on master right after this merges.
Roughly what it needs:
```xml
<goal>install-node-and-npm</goal>
<nodeVersion>v22.x.x</nodeVersion> <!-- keep in sync with .nvmrc -->
...
<goal>npm</goal> <arguments>ci</arguments>
<goal>npm</goal> <arguments>run build</arguments>
```
Worth also dropping the `${basedir}/node_modules` / `${basedir}/node` clean filesets a second look, and noting that `.npmrc`'s `ignore-scripts=true` (a good default, but an undocumented behaviour change in this PR — please call it out in the description) applies to the Maven-driven install too.
## 2. Why CI did not catch this, and a suggestion
All 12 checks are green, which is misleading here: **no CI job builds this module through Maven at all.**
- `compile-check.yml` runs `-P with-all-connectors -P with-examples`, and `with-all-connectors` in `connectors/pom.xml` does **not** include `grafana-plugin` — it lives in the separate `with-grafana` / `with-grafana-plugin` profiles.
- `Jenkinsfile` runs a bare `mvn clean install`, which also excludes those profiles.
- The green `build` check is `grafana-plugin.yml`, i.e. the npm path this PR just rewrote — it is verifying its own new path.
The Maven path is a genuine blind spot, not a one-off oversight, so it will be missed again the same way. Suggest adding a job (either a step in `compile-check.yml` or a second job in `grafana-plugin.yml`) that runs:
```bash
mvn clean package -Pwith-grafana-plugin -DskipTests -ntp
```
That is cheap, and it is what turns item 1 from "someone has to remember" into "CI says so".
## 3. Two decisions that belong on dev@
Neither of these is something I think you should just decide in the PR:
**`grafanaDependency` `>=9.3.0` → `>=12.3.0`.** Your reasoning is sound and well stated — the `@grafana/*` packages are webpack externals resolved from the host at runtime, so a 13.1.0-only API does not fail the build, it fails in the user's Grafana with no bundled fallback, and declaring a floor we can actually stand behind is more honest than a nominal `>=9.3.0`. But the practical effect is dropping support for Grafana 9/10/11 users of a released Apache plugin, and that is a project-level compatibility call. Please raise it on dev@ and let the PMC settle it; if the answer is "keep a lower floor", your own alternative (pinning the `@grafana/*` packages down instead) is the path.
**The generated `.config/AGENTS/` directory (4 files, ~316 lines).** `create-plugin` now emits AI-agent instruction files (`instructions.md`, `e2e-testing.md`, `skills/*.md`) that tell an agent to fetch grafana.com at runtime and state "Do not modify anything inside the `.config` folder". Checking agent-directed instructions into an ASF repository is new ground, and that last rule sits somewhat awkwardly with ASF expectations about governance of code in the tree. Not objecting — just flagging that it should be a conscious PMC decision rather than something that arrives as a side effect of a toolchain bump. Also worth confirming whether `connectors/grafana-plugin/LICENSE` / `NOTICE` need a pass given the dependency set changed wholesale, even though these are devDependencies and `grafana-plugin/**` is RAT-excluded.
---
Item 1 is the only thing that has to change before this can merge. Items 2 and 3 can be follow-ups if the PMC prefers, though item 2 is small enough that folding it in here is probably easier than tracking it separately.
On the two remaining in-place array mutations in `FromValue.tsx` / `SelectValue.tsx`: agree with your call to leave them. A toolchain migration is the wrong place to change untested UI behaviour, and you flagged them explicitly rather than quietly — a separate PR with tests is the right shape.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Migrate the Grafana plugin build from grafana-toolkit to create-plugin [iotdb-extras]
Posted by "PDGGK (via GitHub)" <gi...@apache.org>.
PDGGK commented on PR #118:
URL: https://github.com/apache/iotdb-extras/pull/118#issuecomment-5162397352
Thanks — item 1 is a real break and I had missed it in exactly the way you describe. Pushed as `c244362`.
**The pom.** `install-node-and-yarn` / Node 16.13.1 / Yarn 1.22.17 → `install-node-and-npm` / Node 22.11.0, and `yarn install` / `yarn build` → `npm ci` / `npm run build`. `ci` rather than `install` so the lockfile stays authoritative and a drifted dependency set fails the build instead of resolving silently. I left a comment on the node version pointing at `.nvmrc` so the two do not drift apart again, and called out in the description that `.npmrc`'s `ignore-scripts=true` applies to the Maven-driven install as well — you were right that it was an undocumented behaviour change.
Verified locally: `mvn clean package -Pwith-grafana-plugin -DskipTests -ntp` is green end to end, 45s, with `antrun:3.1.0:run (test)` executing and six backend binaries plus `module.js` carrying build-time timestamps. One caveat on that run: I needed `GOFLAGS=-buildvcs=false`, which is a workaround for a broken `.git` directory in my own home path, not something CI needs — I deliberately did not put it in the job.
**Item 2.** Added as a second job in `grafana-plugin.yml` rather than a step in `compile-check.yml`, so it stays with the module it covers. I checked your diagnosis and it holds: `with-all-connectors` lists nine modules and `grafana-plugin` is not among them, and the Jenkinsfile's plain `mvn clean install` does not activate `with-grafana-plugin` either.
One thing worth recording from getting that job green: the antrun execution has `<id>test</id>` but is bound to `<phase>package</phase>`, so `-DskipTests` does **not** skip it — the Maven job really does run `backend-compile.sh`. That is fine, and it is why the job needs `setup-go`, but the id is misleading and I would not have caught it by reading the id alone.
**Item 3.** Agreed that both belong to the PMC rather than to this PR, and I will raise them on `dev@` as one thread with two sections — the `grafanaDependency` floor as a compatibility decision, and the `.config/AGENTS/` directory together with the LICENSE/NOTICE question you raised. I will link the thread here.
One observation I am deliberately *not* folding in, since it is pre-existing and this PR does not touch the file: `backend-compile.sh` runs `go get -u github.com/grafana/grafana-plugin-sdk-go@v0.250.0` and `go mod tidy` at build time, so a build can mutate `go.mod`. Mine bumped `magefile/mage` from v1.15.0 to v1.17.2, which I reverted before committing. Happy to open that as its own issue once this lands.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Migrate the Grafana plugin build from grafana-toolkit to create-plugin [iotdb-extras]
Posted by "PDGGK (via GitHub)" <gi...@apache.org>.
PDGGK commented on PR #118:
URL: https://github.com/apache/iotdb-extras/pull/118#issuecomment-5242188813
The workflow runs are through -- 14 checks, all green, including the two this PR
adds.
`minimum-grafana-version` is the one worth a line, since it had never executed
anywhere before: it parses the floor out of `plugin.json`, boots that Grafana
release alongside IoTDB, and checks both that the plugin frontend renders its own
config fields and that a table-model query returns data through the backend. Both
passed, so the declared floor is now held by something that runs rather than by
the declaration alone.
`maven-build` covers the `mvn -Pwith-grafana-plugin` entrypoint, which no other
job was exercising.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Migrate the Grafana plugin build from grafana-toolkit to create-plugin [iotdb-extras]
Posted by "CritasWang (via GitHub)" <gi...@apache.org>.
CritasWang merged PR #118:
URL: https://github.com/apache/iotdb-extras/pull/118
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org