feat(service): add db version info to status response #35
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds server and per-response version metadata: server reads app (embedded) version and DB schema version, logs DB version on startup, and enriches component service responses with Server and Db fields. Dependency upgrades and config addition for APP_VERSION included. Changes
Sequence DiagramsequenceDiagram
participant Client
participant ComponentServer as Component Server
participant Database
participant Telemetry
Client->>ComponentServer: Request (SearchComponents / GetComponentVersions)
activate ComponentServer
ComponentServer->>Database: Query component data
Database-->>ComponentServer: Component data / error
ComponentServer->>Database: Query db_version table (getDBVersion)
Database-->>ComponentServer: DB schema_version / created_at / empty/error
ComponentServer->>Telemetry: Record timing (uses config.App.Version)
Note over ComponentServer: Build StatusResponse with Data, Status, Db{schema_version,created_at}, Server{version}
ComponentServer-->>Client: StatusResponse {Data, Status, Db, Server}
deactivate ComponentServer
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@go.mod`:
- Around line 13-16: The build is failing because go.sum is missing checksums
for the newly added modules (github.com/scanoss/go-grpc-helper,
github.com/scanoss/go-models, github.com/scanoss/go-purl-helper,
github.com/scanoss/papi); run `go mod tidy` in the module root to download and
verify those dependencies and regenerate go.sum, then commit the updated go.sum
so CI can pass.
🧹 Nitpick comments (1)
pkg/service/component_service.go (1)
175-186: Consider passing the request context for better observability.Using
context.Background()loses request tracing and cancellation signals. For consistency with observability practices and to allow the DB query to be cancelled if the request is cancelled, consider passing the context from the caller.♻️ Suggested refactor
-func (d componentServer) getDBVersion() *common.StatusResponse_DB { - dbVersion, err := d.dbVersionModel.GetCurrentVersion(context.Background()) +func (d componentServer) getDBVersion(ctx context.Context) *common.StatusResponse_DB { + dbVersion, err := d.dbVersionModel.GetCurrentVersion(ctx) if err != nil || len(dbVersion.SchemaVersion) == 0 { return nil } return &common.StatusResponse_DB{ SchemaVersion: dbVersion.SchemaVersion, CreatedAt: dbVersion.CreatedAt, } }Then update all call sites to pass
ctx:status.Db = d.getDBVersion(ctx)
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
pkg/cmd/server.go (2)
24-32:⚠️ Potential issue | 🟠 MajorUpdate go.sum: missing entries for github.com/scanoss/go-models and github.com/scanoss/go-grpc-helper
The go.mod file declares both
github.com/scanoss/go-models v0.3.0andgithub.com/scanoss/go-grpc-helper v0.11.0in the require block, but go.sum is missing the corresponding checksums for these modules. Rungo mod tidyto sync go.sum with go.mod.
19-31:⚠️ Potential issue | 🟡 MinorSuppress ErrTableNotFound noise in startup DB version logging.
The changelog for v0.7.0 explicitly states "Log error when querying db version fails with an error other than
ErrTableNotFound", but the current code at lines 116-125 warns on any error. The codebase already uses the correct pattern elsewhere (e.g.,pkg/service/component_service.go).🐛 Suggested fix
import ( "context" _ "embed" + "errors" "flag" "fmt"defer gd.CloseDBConnection(db) // Log database version info dbVersionModel := gomodels.NewDBVersionModel(db) dbVersion, dbVersionErr := dbVersionModel.GetCurrentVersion(context.Background()) if dbVersionErr != nil { - zlog.S.Warnf("Could not read db_version table: %v", dbVersionErr) + if !errors.Is(dbVersionErr, gomodels.ErrTableNotFound) { + zlog.S.Warnf("Could not read db_version table: %v", dbVersionErr) + } } else if len(dbVersion.SchemaVersion) > 0 {
🧹 Nitpick comments (2)
CHANGELOG.md (1)
8-22: Consider clearing the “Upcoming changes...” placeholder under Unreleased.
Optional doc cleanup now that 0.7.0 is published.Also applies to: 42-42
pkg/service/component_service.go (1)
174-191: Prefer request context in DB version lookup for cancellation/log context.
Right now it always usescontext.Background(), which drops request-scoped metadata. Consider passingctxintogetDBVersionand using it for DB calls and logging.♻️ Suggested update
-func (d componentServer) getDBVersion() *common.StatusResponse_DB { - dbVersion, err := d.dbVersionModel.GetCurrentVersion(context.Background()) +func (d componentServer) getDBVersion(ctx context.Context) *common.StatusResponse_DB { + if ctx == nil { + ctx = context.Background() + } + dbVersion, err := d.dbVersionModel.GetCurrentVersion(ctx) if err != nil { if !errors.Is(err, gomodels.ErrTableNotFound) { - s := ctxzap.Extract(context.Background()).Sugar() + s := ctxzap.Extract(ctx).Sugar() s.Errorf("Failed to get db version: %v", err) } return nil }Update callers to pass the request
ctx(e.g.,status.Db = d.getDBVersion(ctx)).
isasmendiagus
deleted the
feat/SP-3636/add-db-version-to-status-response
branch
Summary
Dependencies
This PR depends on the following PRs being merged and tagged with their next minor version:
Summary by CodeRabbit
New Features
Tests
Chores
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.