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

Show a useful error message if you try to build lambdas on MacOS #11941

Closed
benjyw opened this issue Apr 18, 2021 · 7 comments · Fixed by #13790
Closed

Show a useful error message if you try to build lambdas on MacOS #11941

benjyw opened this issue Apr 18, 2021 · 7 comments · Fixed by #13790

Comments

@benjyw
Copy link
Contributor

benjyw commented Apr 18, 2021

Right now it fails in all sorts of subtle ways relating to failures to resolve requirements and picking weird interpreters (e.g., due to the issue explained in pex-tool/pex#957).

It hypothetically possible to build lambdas on MacOS if all transitive requirements have prebuilt linux wheels, but that is too narrow a case to support, and it's subtle and tricky to get right even if we wanted to. Better to just say no in a helpful and explicit way.

@riisi
Copy link
Contributor

riisi commented Nov 22, 2021

@benjyw Started looking at this. Were you thinking a logger.error message or raising an exception - I'm not sure what the convention is elsewhere.

@kaos
Copy link
Member

kaos commented Nov 22, 2021

@riisi I think you want to raise an exception, in order to not proceed with the build.
Edit: including the helpful message in the exception..

@riisi
Copy link
Contributor

riisi commented Nov 22, 2021 via email

@Eric-Arellano
Copy link
Contributor

I think you could use FallibleProcessResult instead of ProcessResult so that the error is not eager, and then inspect the error code and error message to determine dynamically if you should mention macOS as a culprit?

Or as a first step, you could always include that macOS warning if the user is on macOS, but use "hedge words"—like "may" or "it's possible"—to make clear that that is only one possible reason the build failed.

@benjyw
Copy link
Contributor Author

benjyw commented Nov 22, 2021

I don't think parsing error messages is great, there are so many different errors that can occur. On error we can check the platform and mention MacOS as a likely culprit.

@riisi
Copy link
Contributor

riisi commented Dec 2, 2021 via email

@Eric-Arellano
Copy link
Contributor

Yeah, build_pex would need to be modified to use FallibleProcessResult, which is almost never what we want. It's convenient that the result is a built PEX and callers don't need to handle the failure case.

Instead, I think I still recommend this:

Or as a first step, you could always include that macOS warning if the user is on macOS, but use "hedge words"—like "may" or "it's possible"—to make clear that that is only one possible reason the build failed.

Basically use logger.warning() with a variant of the warning info box at https://www.pantsbuild.org/docs/awslambda-python#step-3-run-package.

(We assume some users do not read the docs. Our UX and error messages should be good enough that you can figure things out on your own. At least, that's the aspiration!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants