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

Changes in codec interfaces #16726

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 9 additions & 2 deletions pkg/client/unversioned/testclient/fixture.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,13 @@ type ObjectRetriever interface {
Add(runtime.Object) error
}

// ObjectScheme abstracts the implementation of common operations on objects.
type ObjectScheme interface {
runtime.ObjectCreater
runtime.ObjectCopier
runtime.ObjectTyper
}

// ObjectReaction returns a ReactionFunc that takes a generic action string of the form
// <verb>-<resource> or <verb>-<subresource>-<resource> and attempts to return a runtime
// Object or error that matches the requested action. For instance, list-replicationControllers
Expand Down Expand Up @@ -119,7 +126,7 @@ func AddObjectsFromPath(path string, o ObjectRetriever, decoder runtime.Decoder)
type objects struct {
types map[string][]runtime.Object
last map[string]int
scheme runtime.ObjectScheme
scheme ObjectScheme
decoder runtime.ObjectDecoder
}

Expand All @@ -135,7 +142,7 @@ var _ ObjectRetriever = &objects{}
// as a runtime.Object if Status == Success). If multiple PodLists are provided, they
// will be returned in order by the Kind call, and the last PodList will be reused for
// subsequent calls.
func NewObjects(scheme runtime.ObjectScheme, decoder runtime.ObjectDecoder) ObjectRetriever {
func NewObjects(scheme ObjectScheme, decoder runtime.ObjectDecoder) ObjectRetriever {
return objects{
types: make(map[string][]runtime.Object),
last: make(map[string]int),
Expand Down
79 changes: 38 additions & 41 deletions pkg/runtime/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,61 +20,65 @@ import (
"io"
)

// ObjectScheme represents common conversions between formal external API versions
// and the internal Go structs. ObjectScheme is typically used with ObjectCodec to
// transform internal Go structs into serialized versions. There may be many valid
// ObjectCodecs for each ObjectScheme.
type ObjectScheme interface {
ObjectConvertor
ObjectTyper
ObjectCreater
ObjectCopier
}

// ObjectCodec represents the common mechanisms for converting to and from a particular
// binary representation of an object.
type ObjectCodec interface {
ObjectEncoder
// Codec defines methods for serializing and deserializing API objects.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry - I changed the order of definitions in this file, because it was difficult to follow there.

So summarize the changes:

  • moved ObjectScheme to fixture.go - it is used only there and I think we should remove it completely
  • removed ObjectEncoder interface completely (merged with ObjectCodec)
  • added some TODOs for methods/interfaces that I think should be removed
    Please take a look if you agree with that.

type Codec interface {
Decoder
Encoder
}

// Decoder defines methods for deserializing API objects into a given type
type Decoder interface {
Decode(data []byte) (Object, error)
// TODO: Remove this method?
DecodeToVersion(data []byte, version string) (Object, error)
DecodeInto(data []byte, obj Object) error
// TODO: Remove this method?
DecodeIntoWithSpecifiedVersionKind(data []byte, obj Object, kind, version string) error
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method is used to default the object to the version/kind deduced from the URL it's sent to. The parameter kind and version is deduced from the URL.
https://github.com/kubernetes/kubernetes/blob/master/pkg/apiserver/resthandler.go#L331

We need this method but perhaps we can move it to somewhere else.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know what it is used to - I added it TODO based on @lavalamp opinion that Codec shouldn't do any conversion (which I agree with).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. Thanks for the explanation. In that case, DecodeInto() should be removed as well, because if the version in data is different from that of the obj, it will do the conversion.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be clear, this is a long-term goal and we have a ways to get there.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@caesarxuchao DecodeInto could instead return an error if the provided object is of the wrong type.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But yes. Codec parameters should not be referencing group/version/kind. I didn't stop DecodeIntoWithSpecifiedVersionKind because we needed to make progress.


// TODO: Add method for processing url parameters.
// DecodeParametersInto(parameters url.Values, obj Object) error
}

// Encoder defines methods for serializing API objects into bytes
type Encoder interface {
Encode(obj Object) (data []byte, err error)
EncodeToStream(obj Object, stream io.Writer) error
}

// Codec defines methods for serializing and deserializing API objects.
type Codec interface {
Decoder
Encoder
// TODO: Add method for processing url parameters.
// EncodeParameters(obj Object) (url.Values, error)
}

// ObjectCopier duplicates an object.
type ObjectCopier interface {
// Copy returns an exact copy of the provided Object, or an error if the
// copy could not be completed.
Copy(Object) (Object, error)
}
// ObjectCodec represents the common mechanisms for converting to and from a particular
// binary representation of an object.
// TODO: Remove this interface - it is used only in CodecFor() method.
type ObjectCodec interface {
Decoder

// ObjectEncoder turns an object into a byte array. This interface is a
// general form of the Encoder interface
type ObjectEncoder interface {
// EncodeToVersion convert and serializes an object in the internal format
// to a specified output version. An error is returned if the object
// cannot be converted for any reason.
EncodeToVersion(obj Object, outVersion string) ([]byte, error)
EncodeToVersionStream(obj Object, outVersion string, stream io.Writer) error
}

// ObjectDecoder is a convenience interface for identifying serialized versions of objects
// and transforming them into Objects. It intentionally overlaps with ObjectTyper and
// Decoder for use in decode only paths.
// TODO: Consider removing this interface?
type ObjectDecoder interface {
Decoder
// DataVersionAndKind returns the version and kind of the provided data, or an error
// if another problem is detected. In many cases this method can be as expensive to
// invoke as the Decode method.
DataVersionAndKind([]byte) (version, kind string, err error)
// Recognizes returns true if the scheme is able to handle the provided version and kind
// of an object.
Recognizes(version, kind string) bool
}

///////////////////////////////////////////////////////////////////////////////
// Non-codec interfaces

// ObjectConvertor converts an object to a different version.
type ObjectConvertor interface {
Convert(in, out interface{}) error
Expand Down Expand Up @@ -103,18 +107,11 @@ type ObjectCreater interface {
New(version, kind string) (out Object, err error)
}

// ObjectDecoder is a convenience interface for identifying serialized versions of objects
// and transforming them into Objects. It intentionally overlaps with ObjectTyper and
// Decoder for use in decode only paths.
type ObjectDecoder interface {
Decoder
// DataVersionAndKind returns the version and kind of the provided data, or an error
// if another problem is detected. In many cases this method can be as expensive to
// invoke as the Decode method.
DataVersionAndKind([]byte) (version, kind string, err error)
// Recognizes returns true if the scheme is able to handle the provided version and kind
// of an object.
Recognizes(version, kind string) bool
// ObjectCopier duplicates an object.
type ObjectCopier interface {
// Copy returns an exact copy of the provided Object, or an error if the
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/exact/exact (deep!)/

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

won't block for this :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks - I will be touching this file anyway very soon - so I will update it then.

// copy could not be completed.
Copy(Object) (Object, error)
}

// ResourceVersioner provides methods for setting and retrieving
Expand Down