fix(backend): add foreign key to delete preprocessed data #3259
Conversation
|
Something seems wrong, I'll look into it in the afternoon. |
...src/main/resources/db/migration/V1.6__add_sequence_entries_preprocessed_data_foreign_key.sql
Show resolved
Hide resolved
|
This is definitely the right way to go! We should probably add a few more foreign keys eventually. But great start and should fix the bug once it passes! |
|
The problem might be that you just need to push an empty commit to this branch because the CI doesn't trigger images to build |
theosanderson
force-pushed
the
delete-processed2
branch
from
06b8610 to
644e738
Compare
|
(I just rebased which will trigger that, hope that's ok) |
|
I'll cherry pick this test from Anya: #3258 |
|
I was referring to many tests failing – a bit surprising but haven't yet thought/look into them deeper |
|
Any progress @chaoran-chen? I have a suspicion that the tests might not properly respect the (new) foreign key constraint. So things are either not created or deleted now without that being expected. Good reason to add those constraints sooner rather than later, now we might have to fix a lot of tests 😬 @fengelniederhammer what are your thoughts as you set up most of the tests here? Hopefully it's only the convenience clients that need small fixes. |
|
I fixed it, I just need get my GPG working again so that I can commit... |
chaoran-chen
force-pushed
the
delete-processed2
branch
from
5952985 to
bec4971
Compare
backend/src/test/kotlin/org/loculus/backend/controller/EndpointTestExtension.kt
Outdated
Show resolved
Hide resolved
This would have made it immediately clear what the reason for the test failures in #3259 was. So good to have for future. Also truncate in one go for simplicity and maybe also performance.
* Add tests * format (cherry picked from commit a0a145e)
This would have made it immediately clear what the reason for the test failures in #3259 was. So good to have for future. Also truncate in one go for simplicity and maybe also performance.
This would have made it immediately clear what the reason for the test failures in #3259 was. So good to have for future. Also truncate in one go for simplicity and maybe also performance.
There was a problem hiding this comment.
See #3278 for a way to avoid similar test confusion in the future. Feel free to merge that PR into this PR before merging this PR.
This would have made it immediately clear what the reason for the test failures in #3259 was. So good to have for future. Also truncate in one go for simplicity and maybe also performance.
resolves #3250, alternative to #3252
preview URL: https://delete-processed2.loculus.org
Summary
This adds a foreign key to
sequence_entries_preprocessed_dataforaccessionandversion.PR Checklist
[ ] All necessary documentation has been adapted.[ ] The implemented feature is covered by an appropriate test.