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

Remove database on schema creation failure #837

Merged
merged 2 commits into from
Mar 20, 2025

Conversation

adombeck
Copy link
Contributor

@adombeck adombeck commented Mar 14, 2025

I noticed that if the schema creation fails (e.g. because the schema is invalid), the database file is still created and authd uses it the next time it's started. That results in authd starting up successfully but in failure of each request which uses the broken database.

Let's avoid that by ensuring that the database file is removed when the schema creation fails.

UDENG-6482

Copy link
Collaborator

@3v1n0 3v1n0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good, any way to simulate this in a test?

@adombeck
Copy link
Contributor Author

I added a test

@adombeck adombeck marked this pull request as ready for review March 19, 2025 12:51
@adombeck adombeck requested a review from a team as a code owner March 19, 2025 12:51
@3v1n0
Copy link
Collaborator

3v1n0 commented Mar 19, 2025

Did you push it? 🤔

I noticed that if the schema creation fails (e.g. because the schema is
invalid), the database file is still created and authd uses it the next
time it's started. That results in authd starting up successfully but in
failure of each request which uses the broken database.

Let's avoid that by ensuring that the database file is removed when the
schema creation fails.
@adombeck adombeck force-pushed the remove-db-on-schema-creation-failure branch from 5107002 to ccfeb52 Compare March 19, 2025 16:27
@adombeck
Copy link
Contributor Author

Did you push it? 🤔

The pre-push hook failed :/ I pushed it with ccfeb52

Copy link
Collaborator

@3v1n0 3v1n0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, feel free to merge once the nit is addressed :)

@adombeck adombeck force-pushed the remove-db-on-schema-creation-failure branch from ccfeb52 to 7f8f24b Compare March 19, 2025 16:57
@adombeck adombeck merged commit 7111335 into main Mar 20, 2025
15 checks passed
@adombeck adombeck deleted the remove-db-on-schema-creation-failure branch March 20, 2025 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants