Enforce lifecycle_type on apps, droplets, and builds - #5402
WeiQuan0605 wants to merge 4 commits into
Conversation
12637e5 to
df8f659
Compare
| @@ -0,0 +1,13 @@ | |||
| Sequel.migration do | |||
| up do | |||
| alter_table(:apps) { set_column_not_null :lifecycle_type } | |||
There was a problem hiding this comment.
Have you tested this migration on a table which hold the expected amount of datasets? This might result in ACCESS EXCLUSIVE locks on the tables and take a while to scan, leading to service degradation. We should do this in a zero-downtime fashion like done in other migrations i.e. lib/database/bigint_migration.rb
There was a problem hiding this comment.
Thanks for the feedback! Updated the migration to follow the ADD CONSTRAINT ... NOT VALID + VALIDATE CONSTRAINT pattern. Tested locally with db:migrate and db:rollback.
b254237 to
2539415
Compare
…straint The zero-downtime pattern requires SET NOT NULL after VALIDATE CONSTRAINT; without it db.schema still reports allow_null: true causing the spec to fail.
4f17f4b to
e642e4d
Compare
svkrieger
left a comment
There was a problem hiding this comment.
There is a short window between add_contraint NOT VALID, which enforces a lifecycle_type, and setting the default to buildpack, in which inserts from the old code, without specifying a lifecycle_type will fail. Therefore I would suggest the following order:
- SET DEFAULT 'buildpack'
- add_constraint NOT VALID
- VALIDATE CONSTRAINT
- SET NOT NULL + DROP CONSTRAINT
Related to #5067
A short explanation of the proposed change:
Backfills NULL lifecycle_type values via a migration, enforces NOT NULL
constraint, removes fallback logic from models, optimizes AppListFetcher
(subqueries → simple WHERE clause), and optimizes ProcessPresenter eager
loading.
An explanation of the use cases your change solves
Allows lifecycle_type to be determined directly from the column without
joining lifecycle data tables, reducing N+1 queries and simplifying code.
Enables efficient filtering via GET /v3/apps?lifecycle_type=...
I have reviewed the contributing guide
I have viewed, signed, and submitted the Contributor License Agreement
I have made this pull request to the
mainbranchI have run all the unit tests using
bundle exec rakeI have run CF Acceptance Tests