Accept arbitrary types for memos & results, clarify other kwarg types #747
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Memos are usually of type `kopf.Memo` but can be configured to any custom type (in embedded operators mostly). As such, the framework must not rely on any behaviour of memo instances, so it uses `object` as the type which has no methods or properties or fields. However, for the user-facing signatures of handler callbacks, it is declared as `Any` for the same purpose: to accept any specialised custom type. Users are encouraged to put their own classes as annotations of the `memo` kwarg; `kopf.Memo` works as well. The original declaration of handler callbacks with `memo: AnyMemo` was problematic as it was forcing the operator developers to accept `memo` exactly as this type only: `Union[kopf.Memo, object]`, not just `kopf.Memo` alone — which was against the intended usage scenario with arbitrary custom types or the default type. For even stricter control, to not mess with any other value by accident (as `object` would accept anything), it is redeclared as a _new type_ "AnyMemo". For the user-facing embedded operator functions, it is defined as literally `object` to prevent exposing the declared new type `AnyMemo`. The type-casting of pure `object` to redeclared `AnyMemo=object` is done at the earliest possible point internally, hidden from the users. Signed-off-by: Sergey Vasilyev <nolar@nolar.info>
Signed-off-by: Sergey Vasilyev <nolar@nolar.info>
Previously, `Result(object)` was expected from the handlers despite `Any` was essentially meant for the public protocols. `Result` is only needed for internal type-checking — to avoid using any other value instead of the results by accident. Now, `Optional[object]` is specified for the protocols as seen by the users, and it is casted to `Optional[Result]` where needed. There is no need to declare the protocol results as `Any` because any actual type or structure becomes a special case of `Optional[object]` anyway. The latter form allows us to be strict for not using any methods/fields/properties and to make no implicit assumptions on the result type. Signed-off-by: Sergey Vasilyev <nolar@nolar.info>
nolar
changed the title
This was referenced May 8, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Memos are usually of type
kopf.Memobut can be configured to any custom type (in embedded operators mostly). As such, the framework must not rely on any behaviour of memo instances, so it usesobjectas the type which has no methods or properties or fields.However, for the user-facing signatures of handler callbacks, it is declared as
Anyfor the same purpose: to accept any specialised custom type. Users are encouraged to put their own classes as annotations of thememokwarg;kopf.Memoworks as well.The original declaration of handler callbacks with
memo: AnyMemowas problematic as it was forcing the operator developers to acceptmemoexactly as this type only:Union[kopf.Memo, object], not justkopf.Memoalone — which was against the intended usage scenario with arbitrary custom types or the default type.For even stricter control, to not mess with any other value by accident (as
objectwould accept anything), it is redeclared as a new type "AnyMemo". For the user-facing embedded operator functions, it is defined as literallyobjectto prevent exposing the declared new typeAnyMemo. The type-casting of pureobjectto redeclaredAnyMemo=objectis done at the earliest possible point internally, hidden from the users.Similarly, fixes the differences of the
loggerkwarg (now, there iskopf.Loggertype), and of the handler results (were expected as never-exportedOptional[Result], will be expected asOptional[object]and type-casted where needed).fixes #746