OpenSlides / openslides-meta

MIT License
0 stars 12 forks source link

Make schema file idempotent #63

Closed jsangmeister closed 3 weeks ago

jsangmeister commented 6 months ago

@r-peschke with these changes, the schema can be applied multiple times without errors. This is important for the backend so that the entrypoint can simply try to setup the schema without doing any checks first.

r-peschke commented 6 months ago

@r-peschke with these changes, the schema can be applied multiple times without errors. This is important for the backend so that the entrypoint can simply try to setup the schema without doing any checks first.

I'm not quite sure if the backend should apply the schema without checking. If it is idempotent and you'll do this in production, you skipped the migrations. And for development there are make targets to apply the schema on a new database (create-database-with-schema).

jsangmeister commented 6 months ago

I'm not quite sure if the backend should apply the schema without checking. If it is idempotent and you'll do this in production, you skipped the migrations. And for development there are make targets to apply the schema on a new database (create-database-with-schema).

It is just about the dev setup for now, for production we'll have to see how we apply the schema in the future. In the dev setup, this approach is in line with how the datastore currently builds the datastore: In the entrypoint, the schema is simply tried to be applied, regardless of whether or not that's necessary. In https://github.com/OpenSlides/openslides-backend/pull/2315 in the backend, I would like to do the same in the backend: Apply the schema in the entrypoint to make sure that the new schema exists. This is only possible if the schema is idempotent. The make target is of no help, since that would destroy the current database, which contains the old schema.

rrenkert commented 6 months ago

We should discuss the advantages and disadvantages of this PR. Please do not merge!

r-peschke commented 6 months ago

@r-peschke with these changes, the schema can be applied multiple times without errors. This is important for the backend so that the entrypoint can simply try to setup the schema without doing any checks first.

I'm not quite sure if the backend should apply the schema without checking. If it is idempotent and you'll do this in production, you skipped the migrations. And for development there are make targets to apply the schema on a new database (create-database-with-schema).

I'm not quite sure if the backend should apply the schema without checking. If it is idempotent and you'll do this in production, you skipped the migrations. And for development there are make targets to apply the schema on a new database (create-database-with-schema).

It is just about the dev setup for now, for production we'll have to see how we apply the schema in the future. In the dev setup, this approach is in line with how the datastore currently builds the datastore: In the entrypoint, the schema is simply tried to be applied, regardless of whether or not that's necessary. In OpenSlides/openslides-backend#2315 in the backend, I would like to do the same in the backend: Apply the schema in the entrypoint to make sure that the new schema exists. This is only possible if the schema is idempotent. The make target is of no help, since that would destroy the current database, which contains the old schema.

IMO there exist no use case, where you want to deploy a modified schema to an existing database. This is only useful with a migration, otherwise you may have a corrupt database. If you want to apply a new schema without migration, you have to delete the old data in database. In comparison to the non relational schema, here we always change the physical schema.

jsangmeister commented 5 months ago

I still think that it does no harm and provides benefits for development purposes to make the schema idempotent. If we do not want this, however, we should at least be consistent within the schema - either all commands should be idempotent or none.

r-peschke commented 3 weeks ago

No need for discussion anymore. The relational schema was made to fail if you try to apply it a second time. This could only lead to a corrupt database changing the schema without migrating data.