Skip to content

refactor(controlplane,dataplane): migrate jobs and feature flags from postgres to sqlc#2622

Open
jirevwe wants to merge 7 commits intomainfrom
raymond/refactor/add-sqlc-for-the-rest
Open

refactor(controlplane,dataplane): migrate jobs and feature flags from postgres to sqlc#2622
jirevwe wants to merge 7 commits intomainfrom
raymond/refactor/add-sqlc-for-the-rest

Conversation

@jirevwe
Copy link
Copy Markdown
Collaborator

@jirevwe jirevwe commented Apr 14, 2026

Migrate the last remaining database/postgres/ repository implementations to sqlc-based modules under internal/, completing the postgres-to-sqlc migration.

  • Create internal/jobs/ with sqlc queries, Service implementing datastore.JobRepository (9 methods), and comprehensive tests
  • Create internal/feature_flags/ with sqlc queries across 3 tables (feature_flags, feature_flag_overrides, early_adopter_features), Service implementing both fflag.FeatureFlagFetcher and fflag.EarlyAdopterFeatureFetcher, and comprehensive tests
  • Update all call sites: dataplane worker, server init, API handlers, CLI utilities, and test files
  • Delete database/postgres/job.go, feature_flag.go, feature_flag_fetcher.go, early_adopter_feature_fetcher.go

Migrate the last remaining database/postgres/ repository implementations
to sqlc-based modules under internal/, completing the postgres-to-sqlc
migration.

- Create internal/jobs/ with sqlc queries, Service implementing
  datastore.JobRepository (9 methods), and comprehensive tests
- Create internal/feature_flags/ with sqlc queries across 3 tables
  (feature_flags, feature_flag_overrides, early_adopter_features),
  Service implementing both fflag.FeatureFlagFetcher and
  fflag.EarlyAdopterFeatureFetcher, and comprehensive tests
- Update all call sites: dataplane worker, server init, API handlers,
  CLI utilities, and test files
- Delete database/postgres/job.go, feature_flag.go,
  feature_flag_fetcher.go, early_adopter_feature_fetcher.go
@convoy-engineering
Copy link
Copy Markdown
Contributor

@cursoragent bugbot review

@cursor
Copy link
Copy Markdown

cursor bot commented Apr 14, 2026

The GitHub CLI doesn't have write access to post comments. I'll provide the full review here instead.

Bug Review — PR #2622

Overall assessment: The migration is well-structured and functionally complete. The project compiles cleanly (go build ./... and go vet pass), all old references have been updated, and the sqlc-generated code matches the SQL queries. No critical bugs found.

Issues Found

1. [Medium] ErrJobNotCreated declared but never usedinternal/jobs/impl.go:18 declares this sentinel, but CreateJob uses sqlc's :exec directive which only returns an error, never rows-affected. The old code in database/postgres/job.go checked rowsAffected < 1. Suggestion: remove the dead sentinel or switch to :execresult and add the check.

2. [Medium] feature_flags.Service re-instantiated per HTTP request in handlersapi/handlers/organisation.go calls feature_flags.New(h.A.Logger, h.A.DB) inside 7 different handler methods (lines 292, 382, 428, 488, 544, 603, 915). The old code used stateless package-level functions, so this is a minor allocation regression. Other migrated repos (configRepo, projectRepo) are initialized once. Suggestion: create ffService once at init time, matching worker.go and server.go.

3. [Low] CountPrevJobs SQL has redundant DISTINCT and COALESCEid is a primary key (DISTINCT is a no-op) and COUNT never returns NULL (COALESCE is redundant). Carried over from old code.

4. [Low] Cursor comparison uses string orderingFetchJobsPaginated uses id <= @cursor. Works correctly with ULIDs. Consistent with old code.

Things Done Well

  • Clean compile, no stale references to deleted postgres functions
  • Error sentinels properly consolidated into datastore/models.go
  • Compile-time interface checks for JobRepository, FeatureFlagFetcher, EarlyAdopterFeatureFetcher
  • Pagination logic correctly builds metadata before trimming the extra row
  • Comprehensive test coverage for both packages
  • Consistent structure with other recently migrated modules
Open in Web Open in Cursor 

jirevwe added 6 commits April 15, 2026 15:16
Replace direct instantiation of feature flag service with centralized `FeatureFlagService` in application context. Update relevant API handlers for consistency.
…xplicit

Replaced the default package-level logger (`pkgLogger`) in the postgres module with explicit logger injection. Updated affected functions to accept and use the logger parameter consistently.
- Replace `COALESCE(COUNT(DISTINCT(id)), 0)` with `COUNT(id)` in job count queries for consistency and clarity.
- Remove unused `ErrJobNotCreated` constant from `impl.go`.
- Deleted `postgres_test.go` as it no longer aligns with the current database structure and testing strategy.
…mmands

- Updated `postgres.NewDB` calls to explicitly pass logger instances across CLI utilities and migration commands.
- Replaced package-level log usage with `a.Logger` for consistent logging.
- Improved logging in migration commands (`up`, `down`, `create`) and other utilities.
- Inject logger into CLI initialization and migrate commands for consistent logging.
- Replace `Run` with `RunE` in Cobra commands to standardize error propagation.
- Remove `os.Exit` calls in favor of returning errors for better control flow.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants