-
Notifications
You must be signed in to change notification settings - Fork 0
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
Always use the same database type #391
Comments
thewilkybarkid
added a commit
that referenced
this issue
Aug 6, 2021
Rather than allowing the option of using SQLite locally, this forces the use of Postgres. This simplifies both the codebase and improves the build time of both the dev and prod images (as they no longer have to build SQLite from source). Refs #391
Just remembered there's redundant CouchDB code still. |
thewilkybarkid
added a commit
that referenced
this issue
Oct 28, 2021
This change adds a test case for when a preprint cannot be found. The test case is straightforward but requires changes to be able to support it. As there's a strong coupling between the code and the ORM (e.g. through `getFields`), we can't replace the repositories with in-memory versions. Worse, some of the code requires Postgres SQL, rather than the generic type we can use across different databases. As a result, we have to try and use an actual Postgres instance. Doing so makes the tests slower and harder to develop. (Docker solves the problem for CI, but Jest tests should be isolated enough that they can be run locally, such as in the developer's IDE.) To try and reduce the complexity, this uses pg-mem (https://github.com/oguimbal/pg-mem), an in-memory emulation of Postgres. I've not used this before, and the readme clearly labels it as experimental, but it's worked for what I've tried so far. It has a nice rollback feature, which allows resetting the database before each test. There's probably going to be pitfuls, but it seems to be worth continuing. I had to change the codebase to allow passing in pg-mem's customised MikroORM. Unfortunately, there isn't a clean separation between the ORM and its database connection; I'd be much happier just passing in the latter to the app, but this will have to do for now. Refs #388, #391, #419
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Both SQLite and Postgres are supported, but only the latter is used outside of local development. The former is responsible for a lengthy and complicated build process, let alone code complexity, so should be removed.
The text was updated successfully, but these errors were encountered: