bastisawesome / guessinggame_ttv

A Twitch bot to play a word guessing game
BSD 3-Clause "New" or "Revised" License
0 stars 1 forks source link

Fix Database Manager Not Checking Existing Data on Migration #52

Closed bastisawesome closed 1 month ago

bastisawesome commented 1 month ago

Currently the only work done is to make the tests run. However, this PR should address the following concerns:

I expect to revert the current change, but as I had already made and pushed it before realising the bigger issue here, I will leave this as a draft. Future changes will be made swiftly.

bastisawesome commented 1 month ago

The more I review this code, the worse I feel like it is. Every single table houses the schema, then has to run checks to see if there's an old table, grab the old data, then insert it into the new table. This is horribly inefficient. So instead of simply adding the check, I think I'm going to break out the logic into more functions.

bastisawesome commented 1 month ago

Honestly, the more I work on this the more I realise that this entire system is so flimsy! The database manager shouldn't be directly handling migrations. DB migrations should be in their own module and allow incremental updates to the database. So instead of resolving the issue how I intended, I'm thinking the better answer is to have the DBManager class determine if a migration needs to occur, call out to a separate module to run the migration scripts (to be written in the future when a migration occurs), and then resume. This does mean one major change: the in-memory database should not be rebuilding every time it connects.