feat: incremental catalog ingestion on the namespace model (PR #140 rebuilt) - #761
Draft
InduwaraSMPN wants to merge 7 commits into
Draft
feat: incremental catalog ingestion on the namespace model (PR #140 rebuilt)#761InduwaraSMPN wants to merge 7 commits into
InduwaraSMPN wants to merge 7 commits into
Conversation
added 7 commits
August 23, 2026 07:21
…r guards fetchAllPages gains an optional options bag without changing behaviour for existing single-argument callers: - timeoutMs: wall-clock budget for the entire run. Defaults to 60s; 0 disables the budget. One deadline, one setTimeout raced per page, cleared in finally. - signal: AbortSignal cancellation, checked at entry and between pages; the once-listener is removed in finally so it never leaks. - maxPages: opt-in hard cap on pages fetched. Exceeding it throws with the collected item count - it never silently truncates, since a truncated catalog sync looks like a successful one. - Malformed-page guards: a null/undefined page or a page without an items array throws naming the page index and cursor instead of surfacing as a bare TypeError. - Stuck-cursor guard: a server echoing the request cursor back as nextCursor throws instead of looping forever. One guard only. - nextCursor of null or empty string is treated as terminal, matching undefined. Also exports PaginatedResponse and FetchAllPagesOptions from the package barrel. Statement coverage of pagination-utils.ts is 100%; the seven pre-existing tests pass unmodified. Signed-off-by: Induwara <induwara@induwara.com>
Three fetchAllPages closures in OpenChoreoEntityProvider (workflowplanes, observabilityplanes, deploymentpipelines) were written as () => instead of cursor => and omitted the limit query parameter, so those resource types silently ingested only the first server-default page per namespace. Forward the cursor and pass limit: 100 like every other list call in the provider. Note: the four limit: 1000 sites in ci-backend/workflows-backend/ observability POST bodies are NOT changed - the observability query API documents maximum: 1000 for its limit, unlike the OpenChoreo API's cursor-paginated list endpoints which cap at 100. Signed-off-by: Induwara <induwara@induwara.com>
Skeleton of the rebuilt incremental ingestion package on current upstream dependencies (Backstage 1.51 era). Package renamed to @openchoreo/backstage-plugin-catalog-backend-module-openchoreo-incremental matching every sibling; manifest carries repository + configSchema, knex pinned exactly at 3.1.0 like the catalog module, backend-defaults moved to devDependencies, and the unused catalog-backend / permission-common deps dropped. Layout fixes carried in from the analysis: no src/config.d.ts (stale zod shadow), no src/database/migrations (duplicate .ts twins never resolved by the migration runner); migrations/ holds only the .js files that resolvePackagePath actually loads, with a provenance note on the init migration copied from @backstage/plugin-catalog-backend-module-incremental-ingestion. types.ts and config.ts are ported (chunkSize capped at the API's real LimitParam maximum of 100); the provider/engine/database/router internals are compiling stubs throwing 'M3' and are implemented by the following commits. Registered nowhere yet - nothing can break at runtime. Signed-off-by: Induwara <induwara@induwara.com>
The provider's three-level organization/project/component cursor collapses to a two-level namespace walk plus a flat namespace-scoped component fetch, matching the API after components were de-nested from projects. One page per next() call is intentional: the engine persists the cursor between bursts, so fetchAllPages is deliberately not used here. - Phases: namespaces -> projects -> components; done:true only after every namespace's components are ingested. Stuck-cursor guard throws (engine backs off) - the API defines no cursor expiry, so all of the PR's 410 continue-token-expiry machinery is gone. - chunkSize clamped to the documented LimitParam maximum of 100. - Entity translation now imports the sibling plugin's exported translators (translateNamespaceToDomainEntity, translateProjectToEntity, translateComponentToEntity) instead of shipping a local entityTranslator; ComponentTypeUtils comes from backstage-plugin-common via fromConfig(). - ComponentBatchProcessor shrinks to sequential translation; API-entity derivation is deferred until the sibling exports its workload-endpoint helpers. - Engine and WrapperProviders ported verbatim (cursor-agnostic); router ported with two corpus compile bugs fixed (undefined provider params). - Database manager remains a typed stub; M4 ports it. Greps clean: no orgs/, orgName, organization, fetchAllResources, metadata.hasMore/continue, DEFAULT_PAGE_LIMIT, ResponseMetadata, or hardcoded default namespace anywhere in the package. Signed-off-by: Induwara <induwara@induwara.com>
The 1,291-line database manager ports from the original work essentially verbatim, minus one deletion: computeRemoved no longer queries refresh_state/refresh_keys, tables owned by @backstage/plugin-catalog-backend - a cross-plugin schema-ownership violation that would also fail at runtime against this module's schema. The orphaned performance-index migration is reissued as migrations/20240110000001_add_performance_indexes.js, in the one directory the migration runner actually resolves. It indexes only this module's own tables (ingestions, ingestion_marks, ingestion_mark_entities), branches on dialect - CREATE INDEX CONCURRENTLY with transaction:false on Postgres, plain CREATE INDEX IF NOT EXISTS elsewhere - and mirrors down cleanly. Concurrent initialization is now idempotent: WrapperProviders hoists the migration run to a module-level shared promise (the same pattern the sibling module uses for its AnnotationStore), so parallel instances await a single migration run. The database test suite migrates the real migrations directory on in-memory SQLite and covers the ingestion record lifecycle, mark/entity cascade delete, last_error truncation at the expanded width, resume-from-cursor round-trip, double-migration idempotence, parallel WrapperProviders init, and migrate.latest-then-rollback. Signed-off-by: Induwara <induwara@induwara.com>
The incremental module joins the portal composition as one lazy thunk, placed after @openchoreo/backstage-plugin-catalog-backend-module and before @openchoreo/backstage-plugin-backend per the documented ordering constraint, and yields wrapper-first so the extension point registers before its consumer. Config-gating is deliberately two-sided, since the feature loader has no conditional hook: the incremental module's init goes idle unless openchoreo.features.incrementalIngestion.enabled is true (default false - no router, no providers, no migrations), and the sibling catalog module stands the scheduled OpenChoreoEntityProvider down with an explicit log line when the flag is on, so entities are never double-ingested. The feature flag is declared once, in openchoreo-common's config.d.ts alongside the six existing flags - a second declaration of the openchoreo.features path would collide at build time; the incremental package's own config.d.ts declares only openchoreo.incremental tuning keys. app-config.yaml/.production/.local.example carry real keys: flag via OPENCHOREO_FEATURES_INCREMENTAL_INGESTION_ENABLED, tuning defaults burstLength 16s / burstInterval 8s / chunkSize 100 / restLength 60m. README documents enable/disable. Signed-off-by: Induwara <induwara@induwara.com>
124 tests across 11 suites, 90% line coverage, 13s runtime, no CI conditionals, no skipped suites. The core of the suite is a cursor-following client mock (src/testUtils/ mockApi.ts) that walks page chains keyed by nextCursor - the exact multi-page behaviour this package exists for, which the repo's createMockFetchAllPages helper cannot express (it fetches one page and never follows the cursor). The provider suite drives all three phases through >=3-page chains, asserts resume-after-restart by round-tripping the serialized cursor into a fresh provider instance, and pins the stuck-cursor and chunkSize-clamp behaviour. The engine suite covers the rest/ingest/backoff/cancel state machine, burst-length expiry, delta mutations, and the removal-rejection thresholds. The database suite now exercises every manager method's happy path plus representative failure paths on real migrations. WrapperProviders, both catalog modules (via startTestBackend, proving the idle-when-disabled gate), config schemas, validator, tables, errors, and the admin router are covered. Testing surfaced two latent bugs in ConfigValidator, fixed here: its error formatting read zod v3's zodError.errors, which no longer exists in zod 4 (every message degraded to 'Unknown validation error'), and business-rule failures were re-wrapped as CONFIG_VALIDATION_ERROR, swallowing their INVALID_BURST_TIMING / INVALID_API_BASE_URL codes. Signed-off-by: Induwara <induwara@induwara.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
The scheduled
OpenChoreoEntityProvideringests the entire platform in onerun()and emits a singlefullmutation. For large tenants that means along, memory-heavy sync every 300 seconds, and any interruption restarts
from zero. The original PR #140 built a cursor-resumable, burst-based
ingestion package to fix this, but it targeted the organization-era API and
a
continue/hasMorepagination scheme the server never implemented.Why this is still wanted given
EventDeltaApplierUpstream's event-driven delta path handles real-time freshness; the periodic
full sync is now its safety net. What remains uncovered is cold-start
ingestion of large tenants and recovery when the event stream is lossy:
both need a resumable walk that survives restarts, persists progress in the
database, and ingests in bounded bursts. That is exactly this package.
(Maintainer confirmation requested on the PR thread — see open question.)
What this change does
@openchoreo/backstage-plugin-catalog-backend-module-openchoreo-incremental(33→~40 files, ~7,000 lines incl. 124 tests at 90% line coverage).
namespace-scoped component fetch — components are no longer nested under
projects, so the original three-level organization/project/component
cursor collapsed into a redesigned state machine. One page per
next()call; the engine persists the cursor between bursts.
the original 295-line local
entityTranslatorand 244-lineApiErrorHandlerare deleted, superseded by upstream exports.ingestions,ingestion_marks,ingestion_mark_entities) behind knex migrations that run lazily on theplugin database;
migrate.latest+rollbackproven on SQLite and themigration set is idempotent under concurrent init.
openchoreo.features.incrementalIngestion.enabled(declared once, in
openchoreo-common's config schema). When enabled, thescheduled provider in the sibling module stands down with an explicit log
line — no double ingestion. The feature loader has no conditional hook,
so the gate lives inside both modules'
init.LimitParammaximum of 100 (theoriginal PR's 512 was never valid against the real API).
Stack
pr-a/fetch-all-pages-hardening): hardensfetchAllPageswith a timeout budget, abort support, a stuck-cursorguard, a
maxPagescap and malformed-page errors. This PR-B branch isbased on it.
pr-a2/fetch-all-pages-misuse-fixes, optional): fixes threecursor-ignoring
fetchAllPagesclosures in the scheduled provider.(The four
limit: 1000sites named in the analysis were verifiedagainst the observability spec — its query API documents
maximum: 1000, so they are correct and untouched.)What was intentionally dropped from the original PR #140
server repo; CI's freshness gate fails any hand-edit).
continue/hasMorepagination helper andDEFAULT_PAGE_LIMIT = 512(superseded by upstream's cursor protocol and invalid page size).
orgs/-based call sites (the namespace migration rewrote them), the410 continue-token-expiry machinery (the API defines no cursor expiry),
backstage-diff.md, and the fork's commented-out app-config blocks.Migration and rollback story
Three timestamped knex migrations under
migrations/(the init migration isbyte-compatible with Backstage's own incremental-ingestion module, Apache
header and provenance retained).
downrestores the pre-module state onboth SQLite and Postgres. Disabling the feature flag stops the engine; the
tables persist harmlessly and can be dropped with the documented rollback.
Reviewing
One commit per milestone: scaffold → provider rewrite → database layer →
portal wiring → tests.
CODEOWNERSreviewers cc'd. Happy to split alongthose boundaries if preferred.