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.

Sign up for GitHub

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

Jump to bottom

feat: health indicator for Prisma ORM #2250

Merged
merged 9 commits into from
Apr 24, 2023
Merged

feat: health indicator for Prisma ORM #2250

merged 9 commits into from
Apr 24, 2023

Conversation

ThallesP
Copy link
Contributor

@ThallesP ThallesP commented Apr 9, 2023 •
edited
Loading

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Feature

What is the current behavior?

Issue Number: #1510

What is the new behavior?

Add supports for the Prisma ORM

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Currently it only supports SQL based databases, I'm still working on the MongoDB integration.

@BrunnerLivio
Copy link
Member

BrunnerLivio commented Apr 12, 2023 •
edited
Loading

Hey @ThallesP appreciate this PR. The baseline looks good so far. Would it be possible to fix the CI issues as well as adding an e2e test? For the e2e test you can inspire yourself from the mikro orm test

@ThallesP
Copy link
Contributor Author

Hey @ThallesP appreciate this PR. The baseline looks good so far. Would it be possible to fix the CI issues as well as adding an e2e test? For the e2e test you can inspire yourself from the mikro orm test

Yes, no problem, I'm still finishing it (I should've marked this PR as a draft).

@ThallesP ThallesP marked this pull request as draft April 12, 2023 21:07
@ThallesP ThallesP marked this pull request as ready for review April 14, 2023 12:51
@ThallesP
Copy link
Contributor Author

Ready for review, added tests too @BrunnerLivio

ThallesP
ThallesP commented Apr 14, 2023
View reviewed changes
sample/012-prisma-app/package.json
"@nestjs/common": "9.3.2",
"@nestjs/core": "9.3.2",
"@nestjs/platform-express": "9.3.2",
"@nestjs/terminus": "9.2.2",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this should be updated to a newer version with the Prisma changes or else it will fail.

ThallesP
ThallesP commented Apr 14, 2023
View reviewed changes
Copy link
Contributor Author

@ThallesP ThallesP left a comment

Choose a reason for hiding this comment

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

the prisms sample should be updated to a newer version with these PR changes.

@ThallesP
Copy link
Contributor Author

ThallesP commented Apr 16, 2023 •
edited
Loading

@BrunnerLivio prisma only supports v14 and above, that's why the ci/cd failed, might need to bump the node version.
https://www.prisma.io/docs/reference/system-requirements

@BrunnerLivio BrunnerLivio changed the base branch from master to next April 24, 2023 21:43
@BrunnerLivio BrunnerLivio merged commit bef105d into nestjs:next Apr 24, 2023
@BrunnerLivio
Copy link
Member

I merged this PR in the next branch for now, since I can't publish it on the latest distribution because I can't drop support for Node v12 yet.
So this will probably land in Terminus v10. Will continue the discussion over at #1510

@ThallesP
Copy link
Contributor Author

ThallesP commented Apr 24, 2023 •
edited
Loading

@BrunnerLivio technically we don't depend on @prisma/client, if you see I just create an interface that represents the prisma client, so it isn't needed to drop support for node v12, you would need v14+ if someone's project is using the prisma health indicator.

but the ci/cd would be broken as it tests against node v12.

@BrunnerLivio
Copy link
Member

BrunnerLivio commented Apr 25, 2023 •
edited
Loading

@ThallesP I am aware it technically works but I can't drop the Node v12 Pipeline just for Prisma. It's for sure technically possible to not install Prisma or run the Prisma-related tests during the v12 Pipeline but I'd prefer not to jump through these hoops.
Nest v10 is anyway on its way so I prefer to save myself from any short-lived time investment :)

BrunnerLivio pushed a commit that referenced this pull request May 2, 2023
@ThallesP @BrunnerLivio
feat: health indicator for Prisma ORM (#2250)
c41280c
BrunnerLivio pushed a commit that referenced this pull request Jun 16, 2023
@ThallesP @BrunnerLivio
feat: health indicator for Prisma ORM (#2250)
eff611b
BrunnerLivio pushed a commit that referenced this pull request Jun 16, 2023
@ThallesP @BrunnerLivio
feat: health indicator for Prisma ORM (#2250)
6960af6
BrunnerLivio added a commit that referenced this pull request Jun 16, 2023
@BrunnerLivio @ThallesP
v10.0.0 (#2282)
291bbed 
* feat: health indicator for Prisma ORM (#2250)

* chore: drop support for Node v12

BREAKING CHANGE: drop support for Node v12

* chore: fix prisma sample

* chore: add prisma client as optional peer dependency

* refactor(prisma): rename PrismaORM to PrismaHealthIndicator

* chore(): release v10.0.0-beta.0

* chore: fix format

* chore: add node v18 and v20

* chore: drop support for node v14

BREAKING CHANGE:
drop support for node v14

* feat: upgrade to nest v10

* feat(deps): upgrade TypeScript to v5

* chore: update dependencies

* feat(disk): prettify type information

* chore(): release v10.0.0-beta.1

* Revert "feat(disk): prettify type information"

This reverts commit e0b13aa.

* chore: use --legacy-peer-deps in ci

Revert once mikro-orm/nestjs#122 is resolved

* chore: remove prisma timeout

* chore: add debian openssl for prisma

---------

Co-authored-by: Thalles Passos <contato@thalles.me>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ThallesP @BrunnerLivio