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

Epic: ORM for database modeling and migrations #32

Closed
4 of 8 tasks
JamesMGreene opened this issue May 15, 2021 · 12 comments · Fixed by #144
Closed
4 of 8 tasks

Epic: ORM for database modeling and migrations #32

JamesMGreene opened this issue May 15, 2021 · 12 comments · Fixed by #144
Assignees
Labels

Comments

@JamesMGreene
Copy link
Contributor

JamesMGreene commented May 15, 2021

My experiences with Sequelize have all become cautionary tales, so I guided us away from using it at this point and instead favored just using Knex, which is a more basic "SQL query builder".

However, for maintainability's sake, it is probably worthwhile to consider an ORM for the long-term, especially with some contributors not being too familiar with databases.

I would suggest we investigate Prisma as a potential ORM in particular. 🔺

I think it has a lot of nice features that make it a more solid choice than Sequelize. It should also be able to completely replace our Knex usage, as far as I can tell at this point.

Tasks

Status

  • There is a branch created called objection-orm that installs the objection module and sets up the project for creating models.
  • Feel free to create branches off that branch and send PRs to it as the base branch
  • Models should be created in the server/db/models/ directory (only exists in the objection-orm branch)
  • Model filename should be in a singular (not plural) form in all lowercase letters with hyphens as needed, e.g. letter-version.js
  • Model class name should in a singular (not plural) form in PascalCase letters, e.g. LetterVersion

Example, at filename server/db/models/letter-version.js:

const Model = require('./_base')

class LetterVersion extends Model {
  static get tableName() {
    return 'letter_versions'
  }

  // ...additional methods, relationships, etc.
}

module.exports = LetterVersion
@JamesMGreene JamesMGreene added the enhancement New feature or request label May 15, 2021
@manishapriya94
Copy link
Contributor

manishapriya94 commented Jun 30, 2021

Hey all as we were exploring the seeding issue that was coming up in the contributors guideline, some questions came up:

  • @BeeXiong which is the source of truth between local and database production?
  • @teakopp @JamesMGreene @paultela If we're using knex right now would require more SQL knowledge, is it worth it to move to Prisma or Sequelize to speed up this process?

@manishapriya94 manishapriya94 added database and removed enhancement New feature or request labels Jun 30, 2021
@BeeXiong
Copy link
Contributor

  • @BeeXiong which is the source of truth between local and database production?

I don't know which one is the source of truth. Don't think either are actually. Production would be closest I think. I don't think it's wise to switch yet.

I think we need to build out the POST functions to programmatically update the database so that they don't get out of sync.

@JamesMGreene
Copy link
Contributor Author

Forgot to post my updates here.

I looked into Prisma a bit last week, though ended up moving on to work on OpenSourceFellows/amplify-back-end#36 instead for most of the time.

I was super impressed with npx prisma introspect (generates schema and models based on the existing database) and was able to create the schemas for each database (production and local). As we know, they are out-of-sync. I believe we should also be able to generate 2 separate database migrations using Prisma to get each database into our desired state but that's where I need to spend more time investigating what that looks like.

Once I get a little farther and get a better feel for how day-to-day operations would work in a post-Prisma world, I would also love to get some feedback on how folks feel about using the Prisma ORM approach vs. our current Knex query builder approach.

@manishapriya94
Copy link
Contributor

Hey @JamesMGreene was the following idea tied to this?
"Since we are now storing physical/mailing addresses in multiple database tables, we should probably just make a new database table to hold addresses and reference them from the relevant tables (and load them with records via JOIN operations or an ORM).

This would also allow us to validate the addresses more directly for greater data integrity, e.g. require values for at least one line, a city, a state, and a zip code at the database level.

Not an urgent requirement but something that would probably be wise in the long term."

@manishapriya94
Copy link
Contributor

@alwell-kevin for reference

@JamesMGreene
Copy link
Contributor Author

was the following idea tied to this?

Not a hard tie-in but would work well together. 🤷🏻‍♂️

@andyfeller
Copy link
Contributor

@JamesMGreene : I was curious what you've found regarding attaching logic to the models generated around prisma.

looking at https://www.prisma.io/docs/getting-started/setup-prisma/add-to-existing-project/relational-databases/install-prisma-client-typescript-postgres, it seems like the models generated by prisma are generated and updated as prisma commands are called, however I don't get a sense that methods can be attached to these models for business logic that'd you want to use all over the place.

I think https://www.prisma.io/docs/concepts/components/prisma-client/custom-models talks about this but not 💯 sure

@JamesMGreene
Copy link
Contributor Author

JamesMGreene commented Jan 21, 2022

@andyfeller Ah, that's a very interesting observation! 😮

Yes, you appear to correct! It looks like Prisma is only providing static methods for working with the tables/collections, and the objects they return, while capable of including resolving relationships, are just plain objects rather than class instances (a.k.a. models) that could be extended with custom behaviors. 😳

Open feature request: prisma/prisma#5998

Not as ideal as I was imagining, though I'm also not sure that is a deal breaker, per se, given the potential benefits. 🤔

P.S. I see some npm packages for creating Sequelize models from Prisma schemas, which definitely makes me more anxious. 😅

@JamesMGreene JamesMGreene changed the title Investigate Prisma ORM for database modeling and migrations Investigate ORM for database modeling and migrations Feb 4, 2022
@JamesMGreene
Copy link
Contributor Author

Discussed using Objection ORM today in our backend pairing session, which is an ORM based on Knex (which we're currently using), so it might be a nice fit.

cc @waldnzwrld

@manishapriya94 manishapriya94 changed the title Investigate ORM for database modeling and migrations Epic: Investigate ORM for database modeling and migrations Feb 8, 2022
@waldnzwrld waldnzwrld transferred this issue from OpenSourceFellows/amplify-back-end Feb 11, 2022
@waldnzwrld
Copy link
Contributor

I've been using https://bookshelfjs.org/ in another personal project and I like it.

It's maintained and widely used. Can I suggest it?

@manishapriya94
Copy link
Contributor

Thanks @waldnzwrld! @JamesMGreene, the documentation looks great - do we want to focus on this for wednesday?

@JamesMGreene
Copy link
Contributor Author

I've been using https://bookshelfjs.org/ in another personal project and I like it.

I've used Bookshelf in the past and been fine with it. 👌🏻

It's maintained and widely used.

I would agree that Bookshelf is relatively widely used, though not as widely used as Objection. 👌🏻

However, on the point of Bookshelf being maintained, I definite disagree. 😕

Its latest published version on npm is almost 2 years old and its number of open issues are fairly high:

Bookshelf Objection
Last published 2020-06-07 (~21 months ago) 2021-12-30 (~3 months ago)
Open issues 222 63 (~72% fewer)
Weekly downloads 73,505 114,214 (~55% more)

As far as usage ergonomics go, I found Bookshelf's APIs (e.g. fetching records) to feel a bit unnatural. Using the bookshelf-modelbase plugin helps with that, IMHO.

Objection's APIs feel a bit better than Bookshelf's out-of-the-box, IMHO, and look like they are probably more capable of advanced usage in the long haul (through relationships, various JOIN types, etc.).

Given our project's current needs, I think either one could work fine. 👍🏻

If I had to pick one, I'd probably start with Objection. 🤷🏻‍♂️

@manishapriya94 manishapriya94 changed the title Epic: Investigate ORM for database modeling and migrations Epic: ORM for database modeling and migrations Mar 22, 2022
@JamesMGreene JamesMGreene linked a pull request Mar 23, 2022 that will close this issue
@manishapriya94 manishapriya94 added this to the climate can't wait milestone Apr 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Status: Todo
Development

Successfully merging a pull request may close this issue.

5 participants