Add interface / base class for interceptors

[JordonPhillips]

This introduces the base class for interceptors and the necessary helper classes.

Potentially the most questionable part of this is the context object. I've used @property to make the static properties "immutable" since we don't want people changing them outside of the methods designated for that. Of course nothing is stopping them from just mutating properties on the target objects. The question is, should I just give up even this small attempt at commanding the tide?

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

[nateprewitt]

A couple minor comments but otherwise looks good.

[jonemo]

A couple of comments on wording in the docstrings, plus one question about the Exception annotation for response.

It's not quite clear to me what an actual implementation of the interceptor interface would look like that passes mypy muster and doesn't have to repeat much of the complexity of the type annotations from this file. For example, if I write an interceptor that implements two hooks and my messages are NamedTuples that serialize to str, would I have to write out all the following just for the type annotations?

class MyInterceptorContext(InterceptorContext[NamedTuple, NamedTuple, str, str]):
    pass

class MyInterceptor(Interceptor[NamedTuple, NamedTuple, NamedTuple, NamedTuple]):
    def read_before_execution(
        self, context: InterceptorContext[NamedTuple, None, None, None]
    ) -> None:
        do_some_stuff()

    def modify_before_completion(
        self,
        context: InterceptorContext[NamedTuple, NamedTuple, str | None, str | None],
    ) -> NamedTuple | Exception:
        do_some_stuff()

(Apologies if I am missing one or more points in this review. I used this PR review as one way of familiarizing myself with this repo and our spec for this.)

[JordonPhillips]

[...] would I have to write out all the following just for the type annotations? [...]

Yes. We could reduce the type signature size somewhat by making type aliases (for every operation) and moving the nullability into the base signature of the context object. Then you'd have something like:

FooOperationInterceptorContext: TypeAlias = InterceptorContext[NamedTuple, NamedTuple, str, str]

FooOperationInterceptor: TypeAlias = Interceptor[NamedTuple, NamedTuple, NamedTuple, NamedTuple]

class MyInterceptor(FooOperationInterceptor):
    def read_before_execution(
        self, context: FooOperationInterceptorContext
    ) -> None:
        do_some_stuff()

    def modify_before_completion(
        self,
        context: FooOperationInterceptorContext,
    ) -> NamedTuple | Exception:
        do_some_stuff()

You're not necessarily saving much space with this, particularly since most customers aren't likely to write many of them i the first place. What space you save may be lost by having to have an is None check in the body of the method even when the property is unambiguously present. That then also imposes a runtime penalty to what was previously only an import time concern. This also means that customers could generally have unintentional dead code in a number of hooks because the type system is telling them it could be there.

[nateprewitt]

Can we get the typo fixed, other than that this looks good. I left a quick comment on potential exceptions for early access to some of the properties.

I think the @Property approach is at good first step. I'll need to find the time to look at this but it would be nice if we could have some proxy object returned during the read steps that is discarded to avoid mutating.

That may not be feasible with copying/initialization, but it would be nice to at least get benchmarking on it.

[nateprewitt]

Doesn't need to happen in this PR, but this should probably raise an exception on premature access to make it clear it's accessed too early.

[jonemo]

LGTM (pending the one typo): Implements the spec and this provides useful type check time guardrails for implementers of interceptors. I am curious how what the tradeoff between utility of these guardrails and the complexity they introduce will play out when actually writing interceptors, we'll see that in future PRs.

[nateprewitt]

I think we're still waiting on the typo fix here.

[nateprewitt]

🚀

[nateprewitt]

Hmm, it looks like all the builds are failing. The CI/Typing is because pants doesn't support 3.11? I don't think we've changed that?

The Codegen portion seems to be having a hard time installing CRT.

[nateprewitt]

Not positive if this is the issue, but it appears jobs for 2.7/3.6 started breaking for setup-python about 6 hours ago. I can't find anything else in the stack that seems to have changed overnight. They appear to have updated the latest ubuntu image which we are using though.

• Python Version 2.7 with arch x64 not found actions/setup-python#543
• Python 3.6 x64 on Linux: Error: Version 3.6 with arch x64 not found actions/setup-python#544