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

feat(psl): Validation error when shadowDatabaseUrl is identical to url or directUrl #4831

Merged
merged 5 commits into from May 2, 2024

Conversation

pranayat
Copy link
Contributor

@pranayat pranayat commented Apr 15, 2024

Hi there !

This PR closes prisma/prisma#16628

@pranayat pranayat requested a review from a team as a code owner April 15, 2024 19:29
@pranayat pranayat requested review from laplab and removed request for a team April 15, 2024 19:29
Copy link

codspeed-hq bot commented Apr 15, 2024

CodSpeed Performance Report

Merging #4831 will not alter performance

Comparing pranayat:feat/shadow-same-as-direct (22344d2) with main (612a7f7)

Summary

✅ 11 untouched benchmarks

@Druue
Copy link
Contributor

Druue commented Apr 24, 2024

Hey @pranayat! thanks for the PR :)
I took a look and I'm only getting this validation error to appear when I have all three url properties defined.

Error appears on both url and directUrl here when they're both defined
image

But does not appear at all here when only url and shadowDatabaseUrl are defined - A datasource without url leads to other validation errors :)
image

@Druue Druue self-assigned this Apr 24, 2024
@Druue Druue added the PR: Feature A PR That introduces a new feature label Apr 24, 2024
@Druue Druue self-requested a review April 24, 2024 10:24
@Druue Druue added this to the 5.14.0 milestone Apr 24, 2024
@pranayat
Copy link
Contributor Author

@Druue Ah good catch! I've spotted the issue in my code, shall fix it :)

@pranayat
Copy link
Contributor Author

I've pushed a commit which I hopes passes the CI checks. Ever since I synced my branch with upstream two weeks ago, local builds make my system go out of disk space, so I'm no longer able to run tests locally.
image

@pranayat
Copy link
Contributor Author

@Druue Can review now :)

Copy link
Contributor

@Druue Druue left a comment

Choose a reason for hiding this comment

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

Looks good to me, just one final nit 😅

psl/psl-core/src/validate/datasource_loader.rs Outdated Show resolved Hide resolved
@Druue
Copy link
Contributor

Druue commented Apr 30, 2024

✨ 🎉
image

Copy link
Member

@janpio janpio left a comment

Choose a reason for hiding this comment

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

Late to the party (so feel free to ignore!), but how about adding ... to avoid data loss. to the end of both error messages to give users some explanation why as well?

@pranayat
Copy link
Contributor Author

Late to the party (so feel free to ignore!), but how about adding ... to avoid data loss. to the end of both error messages to give users some explanation why as well?

I'll most certainly not ignore this 😄, could save users a google search.

@pranayat pranayat force-pushed the feat/shadow-same-as-direct branch from 00d73dc to 22344d2 Compare May 1, 2024 19:26
Copy link
Contributor

@Druue Druue left a comment

Choose a reason for hiding this comment

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

Awesome! Thank you so much! :D ❤️

@Druue Druue merged commit 0ec3a6f into prisma:main May 2, 2024
204 checks passed
@pranayat
Copy link
Contributor Author

pranayat commented May 2, 2024

Awesome! Thank you so much! :D ❤️

Thank you for the great review 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: Feature A PR That introduces a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Validation error when shadowDatabaseUrl is identical to url (or directUrl)
3 participants