Rebuild Prisma Client between upgrades #1155

noire-munich · 2020-09-14T08:24:18Z

Addresses issue #1083

peterp · 2020-09-17T12:53:58Z

This covers the first part of the task, which is shutting down the api side, but I think we need to generate a new Prisma client after the upgrade has run!

Let me know if you need me to outline the steps here. :)

noire-munich · 2020-09-22T07:19:14Z

@peterp yes please do outline the steps, I'm still noobing a lot of my way through this x)

thedavidprice · 2020-09-22T16:58:58Z

Hi gents!

Kill vs. Warn in case of server is running

If a running server is a part of the problem, might it be easier to simply:

check if server is running (note: we can do this by port, but users can specify a different port)
if running, refuse to run the upgrade command and message user to first shutdown servers

It's a personal preference, but I've always been a proponent of telling users they need to stop processes themselves instead of automatically doing it for them. Another option here could be to default to "warn if process is running" but have an option to "kill process if running".

Upgrade Prisma and Regenerate Client

It seemed the root of the problem was Prisma package/client in node_modules not being updated correctly, true? In that case, should this command delete node_modules/@prisma and node_modules/.prisma before running upgrades?
For regenerating the Prisma Client, let's pass that as an option as well in case a user prefers to upgrade without running yarn rw db up. E.g yarn rw upgrade --no-client. Make sense?

peterp · 2020-09-23T07:01:35Z

Kill vs. Warn in case of server is running

@thedavidprice Yeah, that makes sense, let's do that instead.

It seemed the root of the problem was Prisma package/client in node_modules not being updated correctly

The problem is that the user needs to regenerate the prisma client.

thedavidprice · 2020-10-05T07:04:28Z

@noire-munich just checking back here. Any chance?

noire-munich · 2020-10-05T09:08:19Z

@thedavidprice nope not yet, I was keeping an eye more on #1154 lately.
Do we need either of them soon?

thedavidprice · 2020-10-05T14:46:23Z

Do we need either of them soon?

We do not. Primarily wanted to make sure you saw the previous comments and they made sense!

re: Setup and #1154
Thanks again for getting the Setup command rolling! Super helpful and working well.

Other than waiting for a reply from Peter (btw, he's on vacation until next week), is there anything else you were waiting on?

I'm replying in that thread now.

peterp · 2020-10-10T13:50:24Z

@noire-munich Sorry, I completely missed your comment asking about the steps:

Let the upgrade command run and complete.
Run yarn rw db up (via exaca)
Run yarn rw dev (which will kill the server if it's running) (via exaca)

@thedavidprice I thought a bit about your comments about warning the user about the running server and making them do it instead, but I think it's a lot more code and a bunch more complicated.

noire-munich · 2020-10-10T15:06:05Z

Hi guys!

Thanks again for getting the Setup command rolling! Super helpful and working well.

No problem \o/ glad about it :)

Other than waiting for a reply from Peter (btw, he's on vacation until next week), is there anything else you were waiting on?

Nope thank you, I'm a bit slow on my contributions lately because I'm focusing on a project RW based that I hope could make it to preprod during Q4 :p

@noire-munich Sorry, I completely missed your comment asking about the steps:

No more time off for you @peterp 😈 , seriously though, not a problem 😋 .

Aside from that, both options were clear, however @thedavidprice I would submit to @peterp latest proposal for a first draft, in the undying spirit of "getting things done" before "making them right" ( ok, wording has changed a bit since I last checked the roadmap :p ).

I'll go with it and make modifications if we are satisfied and want to push it a bit further, if it's ok with you both :)

…ch/redwood into pr/prisma-problems-on-upgrade

…server.

noire-munich · 2020-10-10T16:15:15Z

@peterp I updated the code, I've left two things I'd like to sort out together:

the shutdown command feels irrelevant now that we reboot the dev server entirely, can you confirm? I'd remove it to reduce the time of the upgrade command, but to make things as clean as possible I'd shutdown the api and web sides entirely.
I used the execa.command, it throws, I thought of using maybe execa.commandSync since I don't think we'd do anything with stdout or stderr but I'm not sure this one throws. What do you think about it?

peterp · 2020-10-10T17:07:29Z

The shutdown command feels irrelevant now that we reboot the dev server entirely, can you confirm? I'd remove it to reduce the time of the upgrade command, but to make things as clean as possible I'd shutdown the api and web sides entirely.

It is indeed a bit irrelevant now, since attempting to start the dev-server will shut it down if it's already running.

I used the execa.command, it throws, I thought of using maybe execa.commandSync since I don't think we'd do anything with stdout or stderr but I'm not sure this one throws. What do you think about it?

I would suggest using the runCommandTask that we've got over here, you would just supply yarn rw db up as the whole command.

redwood/packages/cli/src/commands/dbCommands/up.js

Lines 45 to 60 in 60e8907

    
           const success = await runCommandTask( 
        
             [ 
        
               { 
        
                 title: 'Migrate database up...', 
        
                 cmd: 'yarn prisma', 
        
                 args: [ 
        
                   'migrate up', 
        
                   increment && `${increment}`, 
        
                   '--experimental', 
        
                   '--create-db', 
        
                   autoApprove && '--auto-approve', 
        
                 ].filter(Boolean), 
        
               }, 
        
             ], 
        
             { verbose } 
        
           )

thedavidprice · 2021-01-21T19:24:12Z

@noire-munich and @peterp checking in to see if it's worth picking up work on this one? If so, let me know (especially if I can be of help).

github-actions · 2021-02-08T11:05:08Z

📦 PR Packages

Click to Show Package Download Links

Install this PR by running yarn rw upgrade --pr 1155:0.24.0-d5017b8

noire-munich · 2021-02-08T11:05:25Z

@thedavidprice @peterp
I've updated the PR and reduced it to only regenerating the Prisma client thanks to the handler David has posted a couple of days ago, which is now used in the dev command and comes with the replacement of rw db to rw prisma.
I believe it should be enough to fix the issue.

thedavidprice · 2021-02-09T21:27:36Z

@noire-munich Thanks so much! And great to have you back 🚀

@dac09 Given some testing, this seems like a candidate for v0.25. Can you confirm on your end whether we should consider?

dac09

LGTM!

thedavidprice · 2021-02-09T23:18:41Z

I'll take it from here. It's on my list for potentially including in v0.25 release. Thanks!

thedavidprice · 2021-02-10T05:32:41Z

Fixed an outdated import path. Otherwise is working 🚀

Will merge once CI passes.

noire-munich · 2021-02-10T08:20:52Z

@noire-munich Thanks so much! And great to have you back rocket

pleasure is mine ;)
and thanks guys for accepting it :)

…ender-p1 * 'main' of github.com:redwoodjs/redwood: v0.25.0 yarn rw prisma commands now show a little tip (redwoodjs#1772) FIx typo (redwoodjs#1779) Ensure all data is removed during the teardown step (redwoodjs#1714) [v0.25] revert to @prisma/cli; recreate yarn.lock (redwoodjs#1774) use prisma db push for test DB (redwoodjs#1768) Disable --version and -v for prisma alias. (redwoodjs#1767) Rebuild Prisma Client between upgrades (redwoodjs#1155) upgrade Prisma v2.16.1 (redwoodjs#1766) [v0.25] Remove warning if you scaffold a model with a relation (redwoodjs#1757) [v0.25] Scaffolded many-to-many relationship fixes (redwoodjs#1758) add forrest and kim-adeline; update all contribs (redwoodjs#1763) Run clean and then build the js. Fix: applying afterQuery in storybook mocks (redwoodjs#1740) restructure and update Deploy command and change Setup Deploy command template [breaking] (redwoodjs#1747) Fixes prisma command when path has spaces Make tutorial work with workspaces. Add "packages/*" to the template.

pr/prisma-problems-on-upgrade

76c08d1

noire-munich mentioned this pull request Sep 14, 2020

Recurring Prisma problems on RW upgrades. #1083

Closed

pr/prisma-problems-on-upgrade - eslint.

e14720a

peterp changed the title ~~pr/prisma-problems-on-upgrade~~ Rebuild Prisma Client between upgrades Sep 17, 2020

peterp added the topic/cli label Sep 18, 2020

Merge branch 'main' into pr/prisma-problems-on-upgrade

c985c1a

noire-munich added 3 commits October 10, 2020 17:09

Merge branch 'main' into pr/prisma-problems-on-upgrade

ff18a94

Merge branch 'pr/prisma-problems-on-upgrade' of github.com:noire-muni…

5867806

…ch/redwood into pr/prisma-problems-on-upgrade

pr/prisma-problems-on-upgrade - new flow, update prisma + reboot dev …

9b426b6

…server.

pr/prisma-problems-on-upgrade - shutdownServer is obsolete.

86b3d59

noire-munich added 3 commits February 8, 2021 11:55

Merge branch 'main' into pr/prisma-problems-on-upgrade

ee4deb5

Using new helper for prisma client generation.

06fd224

Lint fix.

3457fed

peterp approved these changes Feb 8, 2021

View reviewed changes

dac09 added this to the v0.25.0 milestone Feb 9, 2021

dac09 approved these changes Feb 9, 2021

View reviewed changes

thedavidprice added 3 commits February 9, 2021 21:23

Merge branch 'main' into pr/prisma-problems-on-upgrade

c8580db

update import path

92c5fce

Merge branch 'main' into pr/prisma-problems-on-upgrade

d5017b8

thedavidprice merged commit 99e30f3 into redwoodjs:main Feb 10, 2021

thedavidprice mentioned this pull request Feb 10, 2021

yarn rw upgrade now has flashing Listr task output #1777

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rebuild Prisma Client between upgrades #1155

Rebuild Prisma Client between upgrades #1155

noire-munich commented Sep 14, 2020

peterp commented Sep 17, 2020

noire-munich commented Sep 22, 2020

thedavidprice commented Sep 22, 2020

peterp commented Sep 23, 2020

thedavidprice commented Oct 5, 2020

noire-munich commented Oct 5, 2020

thedavidprice commented Oct 5, 2020

peterp commented Oct 10, 2020

noire-munich commented Oct 10, 2020

noire-munich commented Oct 10, 2020

peterp commented Oct 10, 2020 •

edited

Loading

thedavidprice commented Jan 21, 2021

github-actions bot commented Feb 8, 2021 •

edited

Loading

noire-munich commented Feb 8, 2021

thedavidprice commented Feb 9, 2021

dac09 left a comment

thedavidprice commented Feb 9, 2021

thedavidprice commented Feb 10, 2021

noire-munich commented Feb 10, 2021

Rebuild Prisma Client between upgrades #1155

Rebuild Prisma Client between upgrades #1155

Conversation

noire-munich commented Sep 14, 2020

peterp commented Sep 17, 2020

noire-munich commented Sep 22, 2020

thedavidprice commented Sep 22, 2020

Kill vs. Warn in case of server is running

Upgrade Prisma and Regenerate Client

peterp commented Sep 23, 2020

thedavidprice commented Oct 5, 2020

noire-munich commented Oct 5, 2020

thedavidprice commented Oct 5, 2020

peterp commented Oct 10, 2020

noire-munich commented Oct 10, 2020

noire-munich commented Oct 10, 2020

peterp commented Oct 10, 2020 • edited Loading

thedavidprice commented Jan 21, 2021

github-actions bot commented Feb 8, 2021 • edited Loading

📦 PR Packages

noire-munich commented Feb 8, 2021

thedavidprice commented Feb 9, 2021

dac09 left a comment

Choose a reason for hiding this comment

thedavidprice commented Feb 9, 2021

thedavidprice commented Feb 10, 2021

noire-munich commented Feb 10, 2021

peterp commented Oct 10, 2020 •

edited

Loading

github-actions bot commented Feb 8, 2021 •

edited

Loading