prisma-client-py icon indicating copy to clipboard operation
prisma-client-py copied to clipboard

fix: use arm64 prisma binaries on arm64 machines

Open jordemort opened this issue 2 years ago • 5 comments

Fetch arm64 binaries when running on an arm64 machine.

This allows prisma-client-py to work on Linux and macOS machines with arm64 CPUs (it actually already worked on macOS because of Rosetta but this lets it use native binaries.)

Throw a RuntimeError if the machine is not arm64 or x86_64 to avoid downloading binaries with the wrong architecture.

Still WIP because Alpine seems broken, but I'm not 100% sure that Alpine was working to begin with; I'll poke at it more tomorrow.

Change Summary

Please summarise the changes this pull request is making here.

Checklist

  • [ ] Unit tests for the changes exist
  • [ ] Tests pass without significant drop in coverage
  • [ ] Documentation reflects changes where applicable
  • [ ] Test snapshots have been updated if applicable

Agreement

By submitting this pull request, I confirm that you can use, modify, copy and redistribute this contribution, under the terms of your choice.

jordemort avatar Jun 03 '22 01:06 jordemort

Ok, I don't think I've done any harm to alpine users here. I listed out the prisma-photongo bucket and tried all the likely binaries:

 <Key: prisma-photongo,prisma-cli-3.13.0-linux-arm64.gz>,
 <Key: prisma-photongo,prisma-cli-3.13.0-linux-x64.gz>,
 <Key: prisma-photongo,prisma-cli-3.13.0-linux.gz>,

None of those run on Alpine, even with gcompat installed.

Not sure how to best go about setting up a test for this. We could use QEMU to run an ARM64 container and run a test in there? It'd probably have to be a bag on the side of the current testing setup though.

jordemort avatar Jun 03 '22 15:06 jordemort

Seems like you need to run blue on src/prisma/binaries/platform.py

iddan avatar Jun 05 '22 10:06 iddan

Yes @iddan is correct. You can run blue by installing and running pre-commit, e.g.

pip install -U -r requirements/dev.txt
pre-commit install
pre-commit run --all-files

None of those run on Alpine, even with gcompat installed.

What is the error message in this case?

Not sure how to best go about setting up a test for this. We could use QEMU to run an ARM64 container and run a test in there? It'd probably have to be a bag on the side of the current testing setup though.

I'm not that fussed about having integration tests for this as I am looking into switching the implementation to directly use the Node CLI as that would allow us to remove all of the custom logic to download binaries. This shouldn't have any effect for users as we could automatically download node on machines that don't have it installed, see https://github.com/RobertCraigie/pyright-python for an example implementation of this behaviour and #22 for tracking.

RobertCraigie avatar Jun 05 '22 13:06 RobertCraigie

Codecov Report

Merging #416 (e1f5f8f) into main (f75bc91) will decrease coverage by 0.09%. The diff coverage is 41.66%.

@@            Coverage Diff             @@
##             main     #416      +/-   ##
==========================================
- Coverage   94.17%   94.08%   -0.10%     
==========================================
  Files         114      114              
  Lines        5824     5832       +8     
  Branches      326      329       +3     
==========================================
+ Hits         5485     5487       +2     
- Misses        300      303       +3     
- Partials       39       42       +3     
Impacted Files Coverage Δ
src/prisma/binaries/platform.py 63.63% <41.66%> (-6.58%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update f75bc91...e1f5f8f. Read the comment docs.

codecov[bot] avatar Jun 05 '22 13:06 codecov[bot]

Is it worth fixing this one up? I just noticed https://github.com/RobertCraigie/prisma-client-py/pull/233 and it seems much more comprehensive.

jordemort avatar Jun 06 '22 13:06 jordemort

It looks like https://github.com/RobertCraigie/prisma-client-py/pull/454 is going to make this completely obsolete, so I'm closing it.

jordemort avatar Aug 21 '22 14:08 jordemort

Hey @jordemort, I'm so sorry I didn't manage to get to your PR before it was superseded. I really appreciate the effort and I hope I can be more responsive if you decide to open another PR!

RobertCraigie avatar Aug 27 '22 21:08 RobertCraigie