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

applyset: introduce abstraction over objects #3077

Merged
merged 1 commit into from
May 5, 2022

Conversation

justinsb
Copy link
Contributor

@justinsb justinsb commented May 2, 2022

This lets us use typed and untyped objects.

This lets us use typed and untyped objects.
@justinsb justinsb requested a review from martinmaly May 2, 2022 22:21
@justinsb
Copy link
Contributor Author

justinsb commented May 2, 2022

cc @apelisse

"k8s.io/apimachinery/pkg/runtime/schema"
)

type ApplyableObject interface {
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems reasonable. I do wonder about some of the prior art interfaces that are similar but not quite the same. for example

runtime.Object has slightly different structure.

I prefer the straightforward simplicity of the ApplyableObject but it is worth checking whether there is a reason to go with some prior art already in place.

Copy link

Choose a reason for hiding this comment

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

Yeah, we typically get this information with the meta.Accessor from the runtime.Object

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, but the intent is that this isn't necessarily a runtime.Object. It is supposed to be easy to take an unstructured object or a typed object and use it though!

@justinsb justinsb merged commit acf99c6 into kptdev:main May 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants