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

add a sample on rds prisma migration #236

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

add a sample on rds prisma migration #236

wants to merge 1 commit into from

Conversation

HarshCasper
Copy link
Member

No description provided.

Copy link
Contributor

@steffyP steffyP left a comment

Choose a reason for hiding this comment

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

Nice sample @HarshCasper! 🚀

I verified that the sample is working, when following the README or executing the script 👍
As someone who hasn't experience with prisma, I have some questions/comments:

  • it would be nice to add a link and/or some general information about prisma in the README
  • Are the files in prisma/migrations required to run the sample? I witnessed that new files are created when I run the sample, so I wonder if these are artifacts from a specific run?
  • When I run the script, I still had to install another lib. I was asked during execution if I want to install, so I guess it's fine. But I wonder if we could install it beforehand as well?
    Your database is now in sync with your schema.
    
     ✔ Generated Prisma Client (4.16.2 | library) to ./node_modules/@prisma/client in 128ms
     
     
     ┌─────────────────────────────────────────────────────────┐
     │  Update available 4.16.2 -> 5.9.1                       │
     │                                                         │
     │  This is a major update - please follow the guide at    │
     │  https://pris.ly/d/major-version-upgrade                │
     │                                                         │
     │  Run the following to update                            │
     │    npm i --save-dev prisma@latest                       │
     │    npm i @prisma/client@latest                          │
     └─────────────────────────────────────────────────────────┘
     Need to install the following packages:
       [email protected]
     Ok to proceed? (y) y
    
  • For the creation of the database you currently do not set the MasterUsername and MasterUserPassword, and instead rely on LocalStack defaults. This is actually not in parity with AWS, which will deny the request if those parameters are not set. We could leave it that way, but be aware it may lead to errors in the future when we improve parity.
    • Anyhow, it would be great to mention the (default) values for username + password.

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