refactor: remove dead legacy DB-config code paths (phase 1)#228
refactor: remove dead legacy DB-config code paths (phase 1)#228vigneshrajsb wants to merge 2 commits into
Conversation
Static-env remote-config regression: ✅ validatedThe cross-repo
Note: the first attempt failed building the github-source services because the local kind node was memory-overcommitted (build jobs bursting past limits); it went green on retry with staggered builds. This is a local-cluster resource limit, not a code issue — build 54 (lifecycle-examples) built its github service fine on identical code. |
| filterGithubRepositoryId?: number | ||
| ) { | ||
| if (!build?.enableFullYaml || !reconciliationResult?.canReconcile) { | ||
| if (!reconciliationResult?.canReconcile) { |
There was a problem hiding this comment.
[P2] Remove the deleted relation from the reconciliation query below
This path now runs without the enableFullYaml guard, but the stale-deploy lookup later in this method still eagerly fetches [build, service, deployable]. Since Deploy.service is removed in this PR, Objection rejects that query with ValidationError: unknown relation service in an eager expression, so deleting or renaming a YAML service fails before cleanup. Please change that graph to [build, deployable] and ideally cover the real relation expression in a regression test.
There was a problem hiding this comment.
Good catch — fixed in 265f759. The stale-deploy lookup now eager-loads [build, deployable] (the removed Deploy.service relation is gone). staleDeploys is only consumed by cleanupDeploy, which never reads deploy.service, so nothing downstream needed the relation. Added a regression test in the stale deploy reconciliation suite asserting the eager expression no longer references service, so a reintroduction would fail CI.
…ation query The reconcileDeletedDeployables stale-deploy lookup still eager-loaded [build, service, deployable]. Since PR removed the Deploy.service relation, Objection rejects that with 'unknown relation service', so deleting or renaming a YAML service failed before cleanup could run. Change the graph to [build, deployable] (staleDeploys is only used for cleanupDeploy, which never reads deploy.service) and add a regression test asserting the eager expression no longer references the removed relation. Addresses review feedback on #228.
Lifecycle migrated fully to YAML-based per-service configuration pulled at build time; the legacy database-config paths are unreachable (every environment is created with enableFullYaml=true and classicModeOnly has no writers). Remove the dead code: - Collapse ~137 enableFullYaml / classicModeOnly conditionals across 19 files to the YAML/deployable path (every `flag ? deploy.deployable.X : deploy.service.X` becomes `deploy.deployable.X`). Remove the flag writes, the enableFullYamlSupport getter, the queue-fingerprint field, the flag from API responses/swagger, and the classic-only helpers. - Remove the DB->deployable bridge (updateOrCreateDeployableUsingDbConfig and friends); deployables are now materialized purely from YAML. A lifecycle.yaml defaultServices/optionalServices entry carrying serviceId now warn-and-skips instead of resolving a DB Service row. - Delete setupDefaultBuildServiceOverrides/createBuildServiceOverride, ServiceService, and the BuildServiceOverride model. - Remove the Deploy.service relation + serviceId field, Build.buildServiceOverrides, and Environment.services/defaultServices/optionalServices relations, stripping the `service` token from every graph-fetch string across the agent/session/build/deploy code (including the stale-deploy reconciliation query and the query_database AI-tool relation allowlist). DB tables/columns are untouched; they are dropped in phase 2 after this deploys cleanly. The Build.services ManyToMany relation and the deploys.serviceId column are deliberately retained until then. lint / ts-check (zero new deterministic errors) / test (271 suites, 2903+ tests) all green. Validated on local Tilt against lifecycle-examples (github/docker/helm, dependencies, optional-service activation, override API) and lifecycle-static-env (cross-repo remote-config references).
44db80a to
8be8972
Compare
Summary
Phase 1 of a two-phase cleanup that removes the dead legacy database-config code paths left over from Lifecycle's migration to YAML-based configuration. Every environment is now created with
enableFullYaml = true,classicModeOnlyhas no writers, and per-service config lives inlifecycle.yamlpulled at build time — so the DB-config branches are unreachable.This PR contains two logically-separate commits, reviewable in sequence:
1.
refactor: collapse enableFullYaml and classicModeOnly dead code pathsCollapses ~137 conditional sites across 19 files to the YAML/deployable path:
build.enableFullYaml ? deploy.deployable.X : deploy.service.Xnow readsdeploy.deployable.X.enableFullYamlSupportgetter, the queue-fingerprint field, and the flag from API responses/swagger.getClassicServiceGroups,getClassicServiceOverrideState,generateResourceRequests/LimitsForService, theconfigurations-table read path).2.
refactor: remove DB->deployable bridge and legacy Deploy.service pointerMakes deployables purely YAML-sourced:
updateOrCreateDeployableUsingDbConfigand friends). Alifecycle.yamldefaultServices/optionalServicesentry carryingserviceIdnow warn-and-skips instead of resolving a DBServicerow.setupDefaultBuildServiceOverrides/createBuildServiceOverride,ServiceService, and theBuildServiceOverridemodel.Deploy.servicerelation +serviceIdfield,Build.buildServiceOverrides, andEnvironment.services/defaultServices/optionalServicesrelations — stripping theservicetoken from every graph-fetch string across the agent/session/build/deploy code.query_databaseAI-tool relation allowlist to drop the removeddeploys.servicerelation.Not in scope (phase 2)
DB tables/columns stay until this deploys cleanly and is verified in production. Phase 2 then drops
services,configurations,services_disks,environmentDefaultServices/environmentOptionalServices,build_service_overrides, the flag columns, anddeploys.serviceId; recreates thedeploySummaryview; and removesserviceIdfrom the YAML schema. TheBuild.servicesManyToMany relation and thedeploys.serviceIdcolumn are deliberately retained until then. A companion one-line lifecycle-cli PR removes the vestigialenableFullYamltype field.Testing
pnpm lint✓,pnpm ts-check(zero new deterministic type errors vs baseline),pnpm test— 271 suites / 2903 tests ✓.lifecycle-examples(github/buildkit + docker + native-helm services, arequiresdependency, an optional service, template variables): full build→deploy cycle reaches deployed with all services ready on the cleaned-up code. Deployables materialize purely from YAML; env-var template resolution ({{{backend_publicUrl}}},{{{cache_internalHostname}}}) and thePATCH /api/v2/builds/{uuid}/servicesoverride path both verified. A pre-change baseline on unmodifiedmainwas captured first for comparison.lifecycle-static-envremote-config-reference path (cross-repodefaultServiceswith explicitrepository/branch) has not yet been exercised end-to-end — recommend a targeted run before merge.Notes for reviewers
deployablepayload.