Skip to content

(WIP) Support for Postgres as an alternative backend to sqlite #136

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

simon-ball
Copy link

Parallel to issue #125

I've been working on supporting Postgresql as an alternative to sqlite as the backend for deployment at NTNU.

Things are mostly working, but I'm a little bit lost in fixing some outstanding aspects. This PR is not ready for merging, but I'm opening it to solicit some assistance in bringing the changes up to a standard to merge back in.

Changes so far:

  • Modifying the database column definitions to accomodate stricter specifications of postgres.
  • Fixing the order of the clear_db to delete child tables before parent tables
  • Support the concept of schemas (defined in the database url) and creation of the schema if not exists
  • Add a switch within the tests to support a postgres backend, with a default user, password, and schema

Outstanding issues:

  • passing tests:
    • Many of the tests, including with sqlite, seem to hit the default time-out period, not sure why. This is resolved with a custom timeout: pytest . --async-test-timeout 10
    • with a postgres backend, the following 4 tests fail with a generic HTTP500 error.
      • test_download_assignment
      • test_delete_assignment
      • test_download_submission
      • test_download_feedback
    • These tests, all file related, all fail in the test suite with http500, i.e. they hide the actual problem, and I just don't understand the way the tests are written well enough to expose what is actually going wrong.
    • They do not fail in non-automated testing (e.g. submitting the same queries manually via interactive Python terminal works correctly)
  • Alembic database migration
    • The alembic migration fails whether the schema exists or not, currently requires running with the --no-upgrade-db argument.
  • Code quality
    • The addition of postgresql to test_ngshare.py needs to be tidied up, but I've left that until I can figure the other issues out.
    • Since postgresql requires a username/password and permission to the test schema, the tester needs to set those up themselves when configuring the pg database for testing.

Original sql definitions conflate text and integer columns, which Postgres does not allow.
Modified to Integers to match (storing numerical ids throughout)
Deletion order of `clear_db` not valid for Postgres (attempts to delete tables while dependents still exist)
sqlite doesn't need a schema to exist first, postgres does. So if the schema (indicated in the db_url) doesn't exist, create it.
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.

1 participant