Remove stored meshes feature from backend and postgres #8554
Conversation
📝 WalkthroughWalkthroughThe changes remove all backend support for persistent mesh storage and management. This includes deletion of mesh-related controller logic, model definitions, database schema, SQL migrations, route configurations, and message keys. Migration scripts are added for both dropping and recreating the relevant tables and views, ensuring clean schema evolution and reversibility. Changes
Assessment against linked issues
Suggested labels
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
conf/evolutions/131-remove-stored-meshes.sql (2)
1-1: Enforce pre-migration schema version
The DO block correctly asserts that the current schema version is 130 before applying this evolution, preventing out-of-order or duplicate migrations.
3-4: Use IF EXISTS for idempotence
Dropping the view and table withoutIF EXISTSmay cause failures if they’ve already been removed. Consider:-DROP VIEW webknossos.meshes_; -DROP TABLE webknossos.meshes; +DROP VIEW IF EXISTS webknossos.meshes_; +DROP TABLE IF EXISTS webknossos.meshes;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
MIGRATIONS.unreleased.md(1 hunks)app/controllers/MeshController.scala(0 hunks)app/models/mesh/Mesh.scala(0 hunks)conf/evolutions/131-remove-stored-meshes.sql(1 hunks)conf/evolutions/reversions/131-remove-stored-meshes.sql(1 hunks)conf/webknossos.latest.routes(0 hunks)tools/postgres/schema.sql(1 hunks)
💤 Files with no reviewable changes (3)
- conf/webknossos.latest.routes
- app/controllers/MeshController.scala
- app/models/mesh/Mesh.scala
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: backend-tests
- GitHub Check: build-smoketest-push
- GitHub Check: frontend-tests
🔇 Additional comments (7)
conf/evolutions/131-remove-stored-meshes.sql (1)
6-6: Bump schema version forward
UpdatingreleaseInformationto 131 aligns the database state with this new evolution.MIGRATIONS.unreleased.md (1)
13-13: Document new Postgres evolution
Including131-remove-stored-meshes.sqlunder “Postgres Evolutions” ensures it’s applied for fresh deployments.tools/postgres/schema.sql (1)
24-24: Initialize to new baseline version
Adjusting the insertedschemaVersionto 131 makes sure clean installs start at the correct migration level.conf/evolutions/reversions/131-remove-stored-meshes.sql (4)
1-1: Verify pre-reversion schema version
The assertion ensures the schema is at 131 before running the rollback, preventing misaligned downgrades.
4-12: Recreatemeshestable schema
The table definition, including its columns, defaults, and checks, matches the original schema prior to removal.
14-18: Re-establish view and foreign key
Recreatingwebknossos.meshes_and re-adding the FK constraint restores the data model for stored meshes.
19-19: Revert schema version back
DowngradingreleaseInformationto 130 correctly resets the migration state after undoing the table removal.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
conf/evolutions/reversions/132-remove-stored-meshes.sql (3)
1-1: Use RAISE EXCEPTION for robust version check
PL/pgSQLASSERTstatements are only active when the server is built with assertions enabled (--enable-cassert). To ensure this check always fires, replace it with an explicitIF … RAISE EXCEPTIONblock.Suggested diff:
-do $$ begin ASSERT (select schemaVersion from webknossos.releaseInformation) = 132, 'Previous schema version mismatch'; end; $$ LANGUAGE plpgsql; +do $$ +begin + IF (SELECT schemaVersion FROM webknossos.releaseInformation) <> 132 THEN + RAISE EXCEPTION 'Previous schema version mismatch: expected 132, found %', (SELECT schemaVersion FROM webknossos.releaseInformation); + END IF; +end; +$$ LANGUAGE plpgsql;
14-14: AvoidSELECT *in view definitions
UsingSELECT *can lead to unexpected results if columns are later added or reordered. Consider listing columns explicitly in the view to guarantee stability over schema changes.
19-19: Scope the schemaVersion update to the expected row
Although there’s typically only one row, adding aWHEREclause guards against unexpected updates if the table ever contains multiple entries:Proposed diff:
-UPDATE webknossos.releaseInformation SET schemaVersion = 131; +UPDATE webknossos.releaseInformation + SET schemaVersion = 131 + WHERE schemaVersion = 132;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
conf/evolutions/reversions/132-remove-stored-meshes.sql(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build-smoketest-push
- GitHub Check: frontend-tests
- GitHub Check: backend-tests
🔇 Additional comments (2)
conf/evolutions/reversions/132-remove-stored-meshes.sql (2)
4-12: Verify table definition matches original schema
Ensure that every column, constraint name, data type (especially thedatacolumn) and default value here exactly mirrors what existed before the drop. Also confirm that thewebknossos.VECTOR3type remains defined and wasn’t removed in the apply migration; otherwise this reversion will fail at runtime.
16-17: Double-check foreign key constraints
Confirm that theDEFERRABLEsetting and constraint name (annotation_ref) exactly match the original table’s constraint. If the apply script used a different deferral or validity mode, this may cause downstream consistency issues.
| <<<<<<< HEAD | ||
|
|
There was a problem hiding this comment.
Oops. I’ll repair it in #8559
User-supplied meshes stored in postgres were already inaccessible since #6960 removed the frontend code two years ago.
This PR removes the relevant code as well and drops the mesh data from postgres.
This does not change how meshfiles or ad-hoc meshing is handled.
Steps to test:
Issues: