search

IMAP-Science-Operations-Center / sds-data-manager

MIT License
0 stars 9 forks source link

DB unique constraint #308

Closed greglucas closed 1 day ago

greglucas commented 6 days ago

Change Summary

Overview

We were running into race conditions when adding multiple items to the database. This will prevent that by raising an integrity error when someone tries to commit a duplicate entry. It is a partial index where we want to keep track of multiple FAILED entries, but we only want one INPROGRESS or COMPLETED job to be in the table at a time.

There were a few updates required on this because of the switch to testing with postgres from sqllite. Partial unique indexes aren't supported in sqlite, and postgres is timezone aware with datetimes (sqlite apparently just stores strings of dates under the hood). This meant that our current tests were failing when switching over to a postgres backend, so I updated all tests so that they should pass with either postgres or sqllite.

This builds upon #307, so only the final commit has the updates that need to be reviewed for this PR.

New Dependencies

greglucas commented 6 days ago

Postgres is available on the ubuntu runners by default https://github.com/actions/runner-images/?tab=readme-ov-file#package-managers-usage which is nice, so we are getting the test cases we want with no skips.

============ 64 passed, 1 xfailed, 2 warnings in 133.40s (0:02:13) =============
maxinelasp commented 2 days ago

Is the change to make unique constraints work split across multiple reviews? Could you explain what each review entails? I see in #309 that you add some code that would hit the unique constraint.

greglucas commented 2 days ago

@maxinelasp, sorry for this confusion with the multiple PRs and commits! To actually use the unique constraints I had to refactor the code even more because we were simultaneously updating and writing in the same function. So I pushed that portion off to the following PR #309. Here we should only focus on the second commit: https://github.com/IMAP-Science-Operations-Center/sds-data-manager/pull/308/commits/df22c8d26a98c286ad47118c3154624be369d613 The first commit is from #307 which is where I did a separate refactor to pass a single session around to all DB work rather than getting new sessions each time.