[prev in list] [next in list] [prev in thread] [next in thread] 

List:       python-distutils-sig
Subject:    Re: [Distutils] PEP 517 - a plan of action
From:       Nick Coghlan <ncoghlan () gmail ! com>
Date:       2017-08-29 7:03:14
Message-ID: CADiSq7e-RYSF7pUDQ7t8Bn0JZaBAAL6ephgw_+fRYS54+j2rYw () mail ! gmail ! com
[Download RAW message or body]

On 27 August 2017 at 21:03, Thomas Kluyver <thomas@kluyver.me.uk> wrote:
> Thanks Paul for providing us with a way to move forwards. I'm in favour
> of what you propose.
>
> On Sun, Aug 27, 2017, at 11:37 AM, Paul Moore wrote:
>> 2. Thomas to complete and publish his library of helper functions for
>> PEP 517 consumers (optional - but I think he said he had such a thing
>> in progress).
>
> I do. It's not entirely up-to-date with the spec, but it can be found
> here:
> https://github.com/takluyver/pep517
>
> My plan is to provide two levels of API:
> 1. Lower-level machinery to easily call PEP 517 hooks in a subprocess
> (this is what I've been writing so far)
> 2. A higher-level 'give me a wheel' interface which identifies the build
> dependencies, installs them into a build environment, and calls the
> build_wheel hook. Alternatively, this could be a separate package.
>
> I'd like to transfer this repo to PyPA once it's a bit more ready -
> assuming PyPA wants it.
>
>> 3. Build PEP 517 interfaces for flit and setuptools.
>
> The work for flit is well out of date, but it can be found here:
> https://github.com/takluyver/flit/tree/toml-config
>
>> 1. On any remaining questions for the PEP, let's just go for the
>> simplest solution for now. Let the PEP authors decide what that is,
>> with Nick as BDFL arbitrating if there's disagreement.
>
> I believe that the remaining questions are:
> - srcdir-on-sys.path
> - Notimplemented/Error (/None)
>
> Unfortunately, it's not clear to me what's the 'simplest' solution on
> either, and we can't easily change whichever decision we make. Nathaniel
> & Nick still disagree on srcdir-on-sys.path, I believe; I'm not sure how
> we resolve that.

I've actually found the arguments in favour of leaving the source
directory off sys.path by default fairly persuasive, as:

1. We have enough problems with Python's default sys.path[0] handling
that I'm entirely sympathetic to the desire to omit it
2. If omitting it is genuinely a problem, we'll likely find out soon
enough as part of implementing a setup.py backend
3. The `tools` section in pyproject.toml already offers a way to
enable bootstrapping of a meta-backend that does `sys.path.insert(0,
os.getcwd())` before loading a bundled backend

So I think we can deem this one resolved in favour of "Frontends must
ensure the current directory is *NOT* on sys.path before importing the
designated backend", as starting there will mean we maximise our
chances of learning something new as part of the initial rollout of
the provisionally accepted API.

> I still think the NotImplemented/Error issue is a prime example of
> bikeshedding. There clearly isn't one 'right' way to do it, because
> intelligent people continue to disagree about it. I'd like somebody to
> make the decision and end the argument, but I'm not sure who is 'above
> the fray' enough to be able to do that.

-------
Note: TL;DR of the below recap is that I'd accept a PEP that used any
of the following three designs:

- backends optionally define an AnticipatedBuildFailure exception for
frontends to catch (preferred)
- anticipated build failures are indicated by raising NotImplementedError
- anticipated build failures are indicated by returning None

I *wouldn't* accept a return value based design that used
NotImplemented as the special return value

So the choice amongst those 3 options then devolves to the folks
actually planning to do the work - while it's my least preferred of
the three, an implemented None-return based API is nevertheless
immeasurably more useful than a purely theoretical exception based
API.
-------

Taking another go at recapping this particular design question from my
perspective:

- the main point of calling the PEP 517 artifact creation hooks is for
their side effect of actually creating the artifact, *not* their
Python return value
- as such, if they fail to create the requested artifact, they should
raise an exception, just like any other state mutation operation
- however, it's possible for artifact creation to fail for anticipated
reasons, which can be signaled by raising a specific kind of exception
describing that reason
- we don't want to couple the API specification to any particular
library, so we either need to choose an existing builtin exception, or
else let the backend declare the exception it will use

In other words, if I call an API to say "create this artifact", and I
don't get an artifact out the other end, then that operation failed.
It may have an anticipated reason for failing, and I may want to
handle those anticipated failures differently from unexpected ones (so
it's worth defining a protocol for distinguishing them), but it still
failed.

In the standard library, probably the best exemplar of this API design
pattern would be the use of UnsupportedOperation in the `io` module
base classes: https://docs.python.org/3/library/io.html#io.UnsupportedOperation

- from the point of view of a running application, what matters about
UnsupportedOperation is that it's an exception: it indicates that the
requested operation failed and can be caught using the regular
exception handlers.
- from the point of view of a developer debugging a failed
application, what matters about UnsupportedOperation is that it
indicates that there's a program bug rather than an environmental
problem.

For PEP 517, the situation is somewhat similar:

- from the point of view of a build pipeline, what matters about
NotImplementedError is that it's an exception: it indicates that the
artifact failed to be created.
- from the point of view of defining fallbacks in a build pipeline,
what matters about NotImplementedError is that it indicates an
anticipated failure mode (such as missing VCS metadata or missing
external dependencies), rather than a generic "we tried it and
something went wrong".

Whether or not that distinction matters will depend on the specific
pipeline (e.g. if you're building for publication to PyPI, the
distinction is irrelevant, and anticipated failures should be treated
the same as any other failure, while if you're building purely for
local installation purposes, you may decide that if an sdist build
fails for anticipated reasons, you will instead fall back to building
a wheel directly)

So I'd still approve a version of the PEP that uses
NotImplementedError to indicate anticipated failures.

I'd also approve a version of the PEP that works similarly to the DB
API (as Jeremy Kloth suggested): rather than picking a reasonable
builtin exception type, we'd instead define a new backend attribute
for "AnticipatedBuildFailure" (which would be required to be a
subclass of "Exception"), and then frontends would catch that for
anticipated failures. While it's slightly more work for frontends to
handle compared to just picking a builtin exception type, this
approach does allow backends to more easily avoid the risk of the
libraries they call inadvertently reporting a nominally anticipated
failure that is actually nothing of the sort.

One added bonus of the exception type declaration approach is that it
would allow backends to explicitly indicate that there *aren't* any
anticipated build failures by skipping defining the
AnticipatedBuildFailure attribute (frontends can substitute in an
empty tuple in that case).

If I'm understanding Donald's perspective correctly, he's defining
failure slightly differently from the way I am: rather then defining
it in terms of "Was the end user's goal (i.e. artifact creation)
achieved?", he's instead defining it in terms of "Is this outcome in
compliance with the interface specification between the API client and
the API implementation?", and requesting that all spec compliant
outcomes be indicated by a return value, and all spec non-complaint
outcomes be indicated by an exception.

While there are cases where that approach does make sense (using None
as a default value for dict.get, using the NotImplemented singleton to
speed up binary operand coercion while still allowing None as the
result of a binary operation), I don't believe those rationales apply
here - the subprocess invocations and any actual artifact creation
will swamp any possible performance implications from raising and
catching an exception, and we don't have any particular need to avoid
requiring wrapping the API call in a try/except statement if an API
client wants to special case the anticipated failures.

That said, while it wouldn't be my preferred outcome, I *would* accept
a version of the PEP that used "None" as an error indicator for the
artifact creation APIs - that's then just a conventional
"Optional[str]" return type, and while I think a structured exception
would be a *better* way of indicating failure to create the requested
artifact (since they provide a way for the backend to provide
information on how to resolve the problem), I wouldn't find the use of
None for this purpose objectionable the way I do the notion of using
the NotImplemented singleton.

Cheers,
Nick.

-- 
Nick Coghlan   |   ncoghlan@gmail.com   |   Brisbane, Australia
_______________________________________________
Distutils-SIG maillist  -  Distutils-SIG@python.org
https://mail.python.org/mailman/listinfo/distutils-sig
[prev in list] [next in list] [prev in thread] [next in thread]