feat(x2a): implement cascading invalidation - #4488
Conversation
Changed Packages
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4488 +/- ##
=======================================
Coverage 61.62% 61.63%
=======================================
Files 2569 2569
Lines 102831 102864 +33
Branches 28778 28809 +31
=======================================
+ Hits 63368 63397 +29
- Misses 37646 37649 +3
- Partials 1817 1818 +1
*This pull request uses carry forward flags. Click here to find out more. Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
|
|
@mareklibra can you check this PR about the migration, LGTM, but I would like to make sure? and also the MCP server, I do not think that we need anything here, but it's good to check. |
| table.string('status').notNullable().defaultTo('pending').checkIn(statuses); | ||
| table.string('phase').notNullable().defaultTo('init').checkIn(PHASES); | ||
| table | ||
| .uuid('project_id') |
There was a problem hiding this comment.
SQLite recreate copies jobs with INSERT … SELECT *, but createJobsTable column order no longer matches the live table (project_id/module_id were moved before error_details in 806b605). That maps error_details → project_id (NOT NULL). Success rows with NULL error_details will fail the migration; other rows will be silently corrupted. Restore the adversarial-migration column order, or use a named-column INSERT. Please add a test that inserts a job, runs this up(), and asserts project_id is unchanged.
| }); | ||
|
|
||
| const lastJob = jobs[0]; | ||
| if (lastJob && JobStatus.from(lastJob.status).isSuccess()) { |
There was a problem hiding this comment.
After this invalidation, getNextPhase / getLastPhaseReached (not updated in this PR) still treat a stale migrate job as “the last phase reached”. For a module waiting on publish, re-running analyze then makes bulk “run next” call publish using stale migrate artifacts. Skip stale the same way cancelled is skipped so the next phase is the first stale/unfinished one. Please add getNextPhase tests for migrate stale → migrate, and migrate+publish stale → migrate.
| } | ||
|
|
||
| return { status: 'pending', errorDetails: undefined }; | ||
| const status: ModuleStatus = JobStatus.from(latestPhaseJob.status).isStale() |
There was a problem hiding this comment.
Mapping stale → success hides the new state at module/project level: ModuleStatusCell still shows the Published chip whenever module.publish exists, and calculateProjectStatus counts the module in neither finished nor waiting. module.statuses.stale translations added in this PR are unused. Please either add stale to ModuleStatus and return it, or compute aggregate status from the earliest stale downstream phase. moduleStatus.test.ts needs cases for this.
| x2aDatabase, | ||
| logger, | ||
| ); | ||
| } catch (err) { |
There was a problem hiding this comment.
Invalidation runs after updateJob has already committed success and errors are only logged. If markJobsAsStale fails, downstream jobs stay success and the UI never shows stale. Please run both updates in one transaction or fail the collectArtifacts callback if invalidation fails so the operator can retry.
| } | ||
|
|
||
| return { status: 'pending', errorDetails: undefined }; | ||
| const status: ModuleStatus = JobStatus.from(latestPhaseJob.status).isStale() |
There was a problem hiding this comment.
moduleStatus.ts now maps stale → success with no new tests, and getNextPhase / ModuleStatusCell / projectStatus / PhasesCard adversarial canRun were not updated for the new status. Please add moduleStatus and getNextPhase cases, plus a collectArtifacts test that an errored downstream job is not marked stale (the 806b605 isSuccess filter).
| '@red-hat-developer-hub/backstage-plugin-x2a': patch | ||
| --- | ||
|
|
||
| invaldating stale jobs - jobs that had a former phase ran again |
There was a problem hiding this comment.
Please use a changelog-ready sentence, e.g. “Mark downstream phase jobs as stale when an upstream phase is re-run successfully.”
| `ALTER TABLE jobs DROP CONSTRAINT IF EXISTS jobs_status_check`, | ||
| ); | ||
| await knex.raw( | ||
| `ALTER TABLE jobs ADD CONSTRAINT jobs_status_check CHECK (status IN ('pending', 'running', 'success', 'error', 'cancelled'))`, |
There was a problem hiding this comment.
down() re-applies a CHECK without 'stale'. On Postgres this fails if any row is already stale; on SQLite INSERT into the restored table will fail the same way. Please convert stale rows first, or document that down is unsupported after this status has been used.
|
|
||
| const previousRunSucceeded = | ||
| !!phase?.status && JobStatus.from(phase.status).isSuccess(); | ||
| const isStale = !!phase?.status && JobStatus.from(phase.status).isStale(); |
There was a problem hiding this comment.
previousRunSucceeded is isSuccess() only, so a stale phase gets the first-run button label (“Run migrate”) plus staleInstructions. Consider treating stale as a previous run (isSuccess() || isStale()) so the CTA stays “Re-run …”.
3 participants
When a phase completes successfully, any finished jobs for downstream phases are automatically marked as stale.