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(get-platform): remove error, rephrase it as a warning #17389

Merged

Conversation

jkomyno
Copy link
Contributor

@jkomyno jkomyno commented Jan 18, 2023

Closes #17390.

  • Stop erroring out when using an unsupported arch when running Prisma on Linux Alpine
  • Add link CLI utility to @prisma/get-platform

TODO

@jkomyno jkomyno changed the title feat: only throw an error when running Prisma on a non-amd64 Alpine without custom compiled prisma engines feat: only throw an error when running Prisma on a non-amd64 Alpine without custom compiled prisma engines Jan 18, 2023
@jkomyno
Copy link
Contributor Author

jkomyno commented Jan 18, 2023

Integration PR: #17392

@jkomyno jkomyno requested a review from aqrln January 18, 2023 10:43
@jkomyno jkomyno marked this pull request as ready for review January 18, 2023 10:43
@jkomyno jkomyno requested a review from Jolg42 as a code owner January 18, 2023 10:43
@jkomyno jkomyno requested a review from a team January 18, 2023 10:43
@Jolg42 Jolg42 added this to the 4.10.0 milestone Jan 18, 2023
@jkomyno
Copy link
Contributor Author

jkomyno commented Jan 24, 2023

Decision points inspired from Slack:

  1. Is this enough to detect whether Prisma is being run via custom compiled engines?
  const usesOfficialEngines = !(
    process.env.PRISMA_QUERY_ENGINE_BINARY ||
    process.env.PRISMA_QUERY_ENGINE_LIBRARY ||
    process.env.PRISMA_MIGRATION_ENGINE_BINARY
  )
  1. It's easier to just output a warning and avoid the error altogether, as users realistically want to use alpine on amd64 (already supported) and arm64 (will be supported by prisma@4.10.0). So, should we remove the error altogether?

  2. Should we conditionally disable the newly introduced warnings in @prisma/get-platforms given some env var, like PRISMA_DISABLE_PLATFORM_RESOLUTION_WARNINGS=1?. Some users wanted that (link), and I could see the reasoning for it

  3. How do we want to frame the warning messages? We should soft them so that the users don't get scared of the new warnings, and ideally link to the docs (which don't exist yet).

…a-engines' of github.com:prisma/prisma into feat/musl-not-amd64-only-fail-when-using-official-prisma-engines
@jkomyno jkomyno changed the title feat: only throw an error when running Prisma on a non-amd64 Alpine without custom compiled prisma engines feat(get-platform): remove error, rephrase it as a warning Jan 25, 2023
Copy link
Member

@Jolg42 Jolg42 left a comment

Choose a reason for hiding this comment

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

@jkomyno Could you remove this binary packages/engines/introspection-engine-darwin-arm64 and force push?

@jkomyno jkomyno enabled auto-merge (squash) January 26, 2023 14:01
@jkomyno jkomyno merged commit 2438177 into main Jan 26, 2023
@jkomyno jkomyno deleted the feat/musl-not-amd64-only-fail-when-using-official-prisma-engines branch January 26, 2023 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants