Db refactor

[user4752]

closes #7

An opinionated database refactor to convert current raw SQL queries to an ORM library (SQLAlchemy) in order for the data returned from the database to be agnostic to the database engine selected, and allow for clarity on which fields are referenced, i.e. object.named_value versus object[random_index].

Will also allow for easier (and automatic) future migrations of the database should the project needs grow, or the database engine of choice change, e.g. sqlite to mysql/mariadb or postgres.

Opinions chosen are:

• table names are the plural form of the data class they contain, i.e. an entry in the servers table is mapped to the Server ORM class.
• all fields are now properly typed for safety with sane defaults to allow cleaner object creation in code.
• all fields are now named to more clearly identify the values to be stored in them should an external editor be used for raw access
• all data entered into the database now follows the soft-delete paradigm, allowing for a basic historical record of the data, i.e. no data is truly deleted, just marked as 'should not be shown in results unless explicitly specified'
• all data contains basic historical record of last value updated timestamp for possible audit purposes.
• all fields referencing a discord value have been set to width of 32, as maximum discord id numbers are 18 in length, and maximum textual identifiers (nicknames) are 32 in length.
• fields using datetime stamps are using the database internal function to generate the timestamp for uniformity; thus all timestamps are in UTC ensuring no timezone 'drift' which could cause confusion. This also allows for displaying time in either server location (hardcoded) / client location (possibly dynamic) timezone without need to worry about the source timezone due to said uniformity.
• caveat to above is an internal datetime which is used by the softdelete extension of the orm library, it forces use of python's datetime function, but thankfully in utc.

Visualization of the new database schema:

Examples of the new database objects:

Todo:

• refactor logic checks which are spread and varying between the calling function, the internals of called function, and the processing of returned data back in the called function to be uniform. and preferably limited to the database class for safety.
• refactor returned datasets to be proper immutable iterator objects (via list() on return) versus returning the iterator object of the database result which can only be iterated once.
• devise a way to allow backups to be done with other database engines to grow the project's options moving forward

Current Caveats:

• do not have a discord to validate all functionality, thus testing has been limited to all functions being tested in isolation; however all code has been confirmed to execute properly and data modification has been validated.

[SocksTheWolf]

Just reading the opinions of the setup here, I'll look through the code either tonight or tomorrow.

While I agree with a lot of the direction here, I am not sure about the soft deletion practice (specifically around servers). Discord data policies require that if a bot is no longer in a server:

• Information about that server is no longer gathered (does not apply to us)
• Any data stored about that server is removed in a timely manner

I couldn't find what is deemed timely, so this cleanup process could just be whenever the bot next does a reconcile operation, or have it on a timer (we can push this in outside of this pr).

[user4752]

While I agree with a lot of the direction here, I am not sure about the soft deletion practice (specifically around servers). Discord data policies require that if a bot is no longer in a server:

• Information about that server is no longer gathered (does not apply to us)
• Any data stored about that server is removed in a timely manner

The soft deletion was meant more as a simplified audit log saying "oh we've interacted with server at least once before" / "we've unbanned this user once before", but it can easy be removed, or actually migrated to a proper audit log. Declaring it as a proper audit log may allow retention to better fall under the 'Applications stated functionality clause' (§5.b ¶2).

Either way, removal is trivial, and would remove one dependency if done so.

[SocksTheWolf]

Okay I've been super busy these last few days so I haven't had a chance to look over stuff. I will try to get some time in early next week at the latest.

I'll try to start the review today.

[user4752]

Thank you very much for this, I apologize for taking so long to get to it. Had a couple unforeseen circumstances come up, so I was unable to review it until now.

Not a worry, I was aware of the craziness of last week and can only imagine the stress it caused.

Most of your comment replies are on my aforementioned todo list; this initial set of commits was to gauge interest, design and compatibility. Will be commenting directly back on the individual issues shortly.

[user4752]

Was there a final opinion on the soft-delete, and/or an opinion on the reported history / audit log (ref #36)? The former to remove if need be, and the later to prep for implementation in the schema as it will directory dictate some of my replies to the review.

[SocksTheWolf]

Was there a final opinion on the soft-delete, and/or an opinion on the reported history / audit log (ref #36)? The former to remove if need be, and the later to prep for implementation in the schema as it will directory dictate some of my replies to the review.

Yes, I don't think it's something we should pursue.

[user4752]

Closing pull request due to conflicts with v1.5.0 release. Will rebase and open new pull request soon.