[Gateway] Add data_version column to gateway-controller artifacts table#2289
Conversation
ashera96
requested review from
AnuGayan,
Arshardh,
CrowleyRajapakse,
HeshanSudarshana,
HiranyaKavishani,
Krishanx92,
PasanT9,
RakhithaRR,
Tharsanan1,
VirajSalaka,
chamilaadhi,
dushaniw,
hisanhunais,
malinthaprasan,
pubudu538,
renuka-fernando,
senthuran16,
tgtshanika,
tharikaGitHub and
tharindu1st
as code owners
|
Warning Review limit reached
Next review available in: 18 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughA new Suggested Reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
gateway/gateway-controller/pkg/storage/sql_store.go (1)
351-358: 🎯 Functional Correctness | 🔵 TrivialVerify
data_version.gomap keys against runtimecfg.Kindvalues.
ArtifactKindis a type alias forstring, so the call compiles without explicit conversion. However,ComputeDataVersionrelies on a static map (dataMinorVersions) keyed by kind strings. The codebase assignscfg.Kindusing a mix of constants (e.g.,models.KindWebSubApi,api.RestAPIKindRestApi) and literals (e.g.,"LlmProxy"). If these string values do not exactly match the map keys, the function silently defaults to version0, bypassing minor version bumps.Inspect
models/data_version.goto confirm the map keys match the string values used in assignments (e.g.,"LlmProxy","RestApi","WebSubApi").🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@gateway/gateway-controller/pkg/storage/sql_store.go` around lines 351 - 358, `ensureDataVersion` is passing `cfg.Kind` straight into `models.ComputeDataVersion`, but the runtime kind strings must exactly match the `dataMinorVersions` map keys in `models/data_version.go`. Review the `cfg.Kind` assignments in the storage flow and normalize any mismatched values so they align with the map entries used by `ComputeDataVersion` (for example `LlmProxy`, `RestApi`, and `WebSubApi`). If needed, update the map keys or the assigned kind strings consistently so `ensureDataVersion` never falls back to version `0` for valid artifact kinds.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@gateway/gateway-controller/pkg/storage/sql_store.go`:
- Around line 351-358: `ensureDataVersion` is passing `cfg.Kind` straight into
`models.ComputeDataVersion`, but the runtime kind strings must exactly match the
`dataMinorVersions` map keys in `models/data_version.go`. Review the `cfg.Kind`
assignments in the storage flow and normalize any mismatched values so they
align with the map entries used by `ComputeDataVersion` (for example `LlmProxy`,
`RestApi`, and `WebSubApi`). If needed, update the map keys or the assigned kind
strings consistently so `ensureDataVersion` never falls back to version `0` for
valid artifact kinds.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1ba53209-0924-49f4-846c-597a1b28435c
📒 Files selected for processing (11)
gateway/build-manifest.yamlgateway/gateway-controller/pkg/models/data_version.gogateway/gateway-controller/pkg/models/data_version_test.gogateway/gateway-controller/pkg/models/stored_config.gogateway/gateway-controller/pkg/storage/gateway-controller-db.postgres.sqlgateway/gateway-controller/pkg/storage/gateway-controller-db.sqlgateway/gateway-controller/pkg/storage/gateway-controller-db.sqlserver.sqlgateway/gateway-controller/pkg/storage/sql_store.gogateway/gateway-controller/pkg/storage/sqlite.gogateway/gateway-controller/pkg/storage/sqlite_test.gogateway/gateway-controller/tests/integration/schema_test.go
ashera96
force-pushed
the
gateway/artifact-data-version
branch
from
3a11551 to
2a2d107
Compare
ashera96
force-pushed
the
gateway/artifact-data-version
branch
from
2a2d107 to
290606d
Compare
| gateway_id TEXT NOT NULL, | ||
| display_name TEXT NOT NULL, | ||
| version TEXT NOT NULL, | ||
| data_version TEXT NOT NULL DEFAULT '1.0', |
There was a problem hiding this comment.
Are we only doing this for artifact table?
cc: @malinthaprasan
There was a problem hiding this comment.
Shall we just blindly do it to all the tables? :)
It doesn't do any harm right. No code changes needed as well. Just a default value with 1.0.
4 participants
Purpose
Adds a
data_versioncolumn to the gateway-controllerartifactstable to record the data-shape version of each stored artifact (REST API, WebSub API, WebBroker API, LLM Provider, LLM Proxy, MCP Proxy).The value is
<major>.<minor>:apiVersionsuffix (gateway.api-platform.wso2.com/v1→1).0(sov1→1.0). Bump it when a kind's stored data shape changes in a backward-compatible way without bumpingapiVersion. Stored rows then record exactly which data shape they were written with, enabling future data migrations.This decouples the wire/API version (
apiVersion, the major) from the stored-data-shape version (the minor).Changes
data_versionadded toartifactsafterversion, matching each dialect's existing style —TEXT NOT NULL DEFAULT '1.0'for SQLite and Postgres,NVARCHAR(20) NOT NULL DEFAULT '1.0'for SQL Server. SQLite schema version bumped3 → 4(PRAGMA user_versionandcurrentSchemaVersion).StoredConfig.DataVersionfield and aGetApiVersion()helper that prefersSourceConfiguration(the user-authored artifact) and falls back toConfiguration(the LLM-derived RestAPI form).ComputeDataVersion(kind, apiVersion)plus an exhaustivedataMinorVersionsmap keyed byArtifactKind.ensureDataVersion()chokepoint inSaveConfig,UpdateConfigandUpsertConfig, so every artifact-addition path records adata_versionand an explicitly-set value is never overwritten.data_versionthreaded through all artifactSELECT/scan paths (single-row reads and the joinedscanConfigRowsqueries).