Skip to content

Move to ORM for database access#89

Merged
theyoyojo merged 13 commits into
masterfrom
orm
Apr 29, 2024
Merged

Move to ORM for database access#89
theyoyojo merged 13 commits into
masterfrom
orm

Conversation

@charliemirabile

Copy link
Copy Markdown
Contributor

Complete overhaul of the database code to replace manually created SQL queries with expressive code using peewee orm.

instead of manually specifying schemata in init-db.sql, we can describe
the fields and their properties as members of classes within a hierarchy
of classes inheriting from peewee orm's base model.

Obviously peewee orm can do a lot more than generate schemata, but this
is a nice (and fully backwards compatible) start on the work towards
fully embracing peewee for all database operations.

The existing queries continue to work because the schemata generated are
fully compatible with the existing ones (albeit not identical, so the
binary blob needed updating).

This means that the existing db code can be slowly phased out in the next
commits and orbit will remain fully functional throughout.

@theyoyojo theyoyojo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

some small comments

Comment thread orbit/radius.py
Comment thread orbit/hyperspace.py
Comment thread orbit/radius.py
Comment thread orbit/hyperspace.py
All calls to the `db.ses_*` functions are replaced by corresponding
code that performs equivalent queries using peewee orm functionality.

The resulting code is both clearer: no need for random [0] on the result,
able to use field names instead of [0], [1], [2] etc; and more expressive:
the logic used is spelled out using normal python syntax like `field ==
value` instead of hidden away in a helper function while not being longer.
All calls to the `db.ses_*` functions are replaced by corresponding
code that performs equivalent queries using peewee orm functionality.

Also includes a minor cleanup on the code that prints the list of
sessions. Printing a header and then no values is as clear or clearer
than printing `(no sessions)` imo, and the code is substantially more
simple as a result.
Now that hyperspace and radius are using peewee orm exclusively for
accessing the sessions table, these functions are unused and can be
removed.
All calls to the `db.reg_*` functions are replaced by corresponding
code that performs equivalent queries using peewee orm functionality.
All calls to the `db.reg_*` functions are replaced by corresponding
code that performs equivalent queries using peewee orm functionality.
Now that hyperspace and radius are using peewee orm exclusively for
accessing the registration table, these functions and their associated
queries are unused and can be removed.
The credentials were being checked very similarly in two places
in the code. Moving the check to a helper means that the code can
be more expressive (early return in function instead of short circuit
that is too long to fit on one line, and self documenting at call sites)
and it means there is one source of truth about how to perform this
critical operation.
All calls to the `db.usr_*` functions are replaced by corresponding
code that performs equivalent queries using peewee orm functionality.
All calls to the `db.usr_*` functions are replaced by corresponding
code that performs equivalent queries using peewee orm functionality.
Now that hyperspace and radius are using peewee orm exclusively for
accessing the user table, these functions and their associated
queries are unused and can be removed.
Now that all db queries are executed using peewee, there is no
need for the old functions for accessing the database or for
the imports that they required.
Now that there are no legacy queries to be backwards compatible with
we do not need to specify the table names explicitly and can let peewee
generate them automatically.

This required updating the binary blob to reflect the new names.

@theyoyojo theyoyojo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm

@theyoyojo theyoyojo merged commit 8a4d98d into master Apr 29, 2024
@theyoyojo theyoyojo deleted the orm branch April 29, 2024 18:01
@charliemirabile charliemirabile mentioned this pull request May 3, 2024
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