Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bugfix(datastore) Fix get_user for various DBs #73

Merged
merged 1 commit into from
May 11, 2019
Merged

Conversation

jwag956
Copy link
Member

@jwag956 jwag956 commented May 11, 2019

DB drivers vary in how strict they are. For example
postgres+pyscopg2 throws errors if you attempt to lower(integer) or
compare a string field with an integer field.
Furthermore, when pyscopg2 throws an internal error like that, it
aborts the transaction - so you can't just try another query.

To solve this - in get_user we check for compatibility of types using
metadata that the ORM provides. This isn't trying to be completely
general - just looking at numeric and non-numeric types - on the
presumption that we would have unique user attributes on things like
dates, boolean, etc!.

To enable testing, added an option to pytest to specify a real DB
url to replace the default sqlite. This required a bit of refactoring
and realized that we really shouldn't be passing in fixtures to
the datastore() fixture since that meant all 5 datastores would be
instantiated EVERY time - which for a real DB was a performance issue.

With this addition - tested Sqlalchemy, peewee, pony with sqlite,
postgres, mysql.

DB drivers vary in how strict they are. For example
postgres+pyscopg2 throws errors if you attempt to lower(<integer>) or
compare a string field with an integer field.
Furthermore, when pyscopg2 throws an internal error like that, it
aborts the transaction - so you can't just try another query.

To solve this - in get_user we check for compatibility of types using
metadata that the ORM provides. This isn't trying to be completely
general - just looking at numeric and non-numeric types - on the
presumption that we would have unique user attributes on things like
dates, boolean, etc!.

To enable testing, added an option to pytest to specify a real DB
url to replace the default sqlite. This required a bit of refactoring
and realized that we really shouldn't be passing in fixtures to
the datastore() fixture since that meant all 5 datastores would be
instantiated EVERY time - which for a real DB was a performance issue.

With this addition - tested Sqlalchemy, peewee, pony with sqlite,
postgres, mysql.
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.

None yet

1 participant