Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make schema file idempotent #63

Conversation

jsangmeister
Copy link
Contributor

@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.

@jsangmeister jsangmeister added the enhancement General enhancement which is neither bug nor feature label Mar 21, 2024
@jsangmeister jsangmeister added this to the 4.2 milestone Mar 21, 2024
@jsangmeister jsangmeister requested a review from r-peschke March 21, 2024 09:11
@r-peschke
Copy link
Member

@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
Copy link
Contributor Author

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.

@rrenkert
Copy link
Member

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

@r-peschke
Copy link
Member

r-peschke commented Mar 25, 2024

@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
Copy link
Contributor Author

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
Copy link
Member

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.

@r-peschke r-peschke closed this Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement General enhancement which is neither bug nor feature needs discussion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants