Strict Decoder / CodecFactory API

[Leigh Capili]

Hello,

Patch https://github.com/kubernetes/kubernetes/pull/74111 adds a feature flag to enable Strict decoding to `runtime.serializer.json`.
Some efforts in Component-Standard WG desire a simple constructor for UniversalDecoders that support Strict behavior.

UniversalDecoders are currently implemented in CodecFactory, but some conversations /w liggit, luxas, and neolit123 about the future of Strict behavior being default (not featureflagged) in apimachinery have set some direction to implement NewUniversalStrictDecoder as a constructor in a separate package.

I've started implementing this in https://github.com/kubernetes/kubernetes/pull/76303 (tests todo), but the implementation requires code duplication from CodecFactory.

I'd appreciate some opinions about how to structure this.
One solution may be to add some features to CodecFactory that we could later deprecate as Strict behavior becomes default.
I've left more detail on the patch.

Cheers.

[Daniel Smith]

General thoughts:

I don't think library-level functionality should be feature-gated, I think it should just be possible to not use it, and it shouldn't auto-opt in existing users of the library. The feature gate should control e.g. whether the user gets a choice or not. (Analogy: the "gate" goes only in the fence, it does not need to be distributed over the entire yard!)

I don't like our habit of encoding options in the name of the constructor and making a ton of different constructors. I'd rather make an explicit options struct.

In particular, "strict or not" sounds like a binary choice that makes sense as an option for all of our decoders; we shouldn't double the number of constructors every time we want to offer such an option.

I haven't yet looked carefully at the code though-- maybe that will give me a different opinion :)

--
You received this message because you are subscribed to the Google Groups "K8s API Machinery SIG" group.
To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-sig-api-m...@googlegroups.com.
To post to this group, send email to kubernetes-sig...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-sig-api-machinery/d265ebb6-f8f7-4ae1-babb-30af02d5a158%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[David Eads]

It seems to me that if this is intended to be an implementable part of apimachinery and you want to provide an implementation, determining what abstractions you need to build it in a separate package and providing those to improve the codebase you're contributing to while making an easily separable unit is a good way to go.  Having a separate package makes it easier to bring in, move out, vet for testing, and improves the condition for whoever wants the next codefactory.  Maybe the next one won't even have to be in tree.

To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-sig-api-machinery/CAB_J3bZD7bW4SeDJLF2gn9V5qS1Q_vKBqjdayS-A5bBFeAHBDw%40mail.gmail.com.