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

Pep517HookCaller's prepare_metadata_for_build_wheel throws subprocess.CalledProcessError instead of something nice #38

Closed
ghost opened this issue Feb 1, 2019 · 8 comments

Comments

@ghost
Copy link

ghost commented Feb 1, 2019

Pep517HookCaller's prepare_metadata_for_build_wheel throws subprocess.CalledProcessError if wheel is not installed, and that it throws an error in that case is of course expected, but wouldn't some sort of other error make more sense? I'm a bit afraid to catch this specific one now since this error seems extremely implementation-specific (related to how it's internally called) rather than problem-descriptive (telling me that wheel is missing), but I don't want to catch Exception either. So I'm not sure how to best catch this sort of problem...?

@takluyver
Copy link
Member

So I think you're asking for the hooks to propagate the Python exception information back to the caller.

That's possible, but I suspect it's not worth the extra complexity. The build backend invoked by this module can run absolutely arbitrary code - extension modules, network requests, even running non-Python subprocesses. By design, the frontend doesn't know what the backend is doing. Thus, if the backend fails, the frontend can't really make sense of what went wrong. An ImportError could be a missing build dependency, or a typo in the code, or some module deciding it needs a plugin which isn't available.

All the frontend can really understand is whether the operation succeeded or failed. If it failed, the only reasonable options are to try a generic fallback (such as running setup.py if it's present), or show the backend output to the user and let them work it out.

@pfmoore
Copy link
Member

pfmoore commented Feb 1, 2019

While it's not @Jonast's use case (and I suspect the requirements are significantly different) there is an advantage in propagating more details of backend errors back to the frontend, which is to provide better error reporting. At the moment, pip gets a BackendUnavailable error when the import of the backend gives ImportError. We can't really do anything more than report that to the user. If we had the ImportError details (at least the traceback) we could print it and give the user a better chance of diagnosing the issue.

See pypa/pip#6164 for an example.

@takluyver
Copy link
Member

That's actually kind of asking for less specific handling of errors. For all 'normal' backend errors, you'll get a CalledProcessError with the output from running the backend, ending with the traceback if there is one. It's only the special-cased BackendUnavailable and UnsupportedOperation errors that don't have that.

But we can probably arrange to include the traceback with those errors.

@ghost
Copy link
Author

ghost commented Feb 1, 2019

@pfmoore a BackendError (with no further details) would already be way better, since that would be distinguishable from some sort of fundamental error of pep517 itself or whatever. (And could be derived from later when more specific errors are ever added, such that code catching BackendError wouldn't suddenly fail)

I really don't care for the traceback at all, my problem is that right now it's kind of designed to make me catch Exception to handle the error and that seems like really bad code

@pradyunsg
Copy link
Member

IIUC, this can be closed correct?

@ghost
Copy link
Author

ghost commented Feb 3, 2019

@pradyunsg was the behavior changed such that a more pep517-specific error is thrown? (like a BackendExecutionError deriving from subprocess.CalledProcessError or similar, to make a concrete suggestion)

@jaraco
Copy link
Member

jaraco commented May 24, 2019

Is this issue a duplicate of #10?

@pradyunsg
Copy link
Member

It is a duplicate of #10.

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

No branches or pull requests

4 participants