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

Default to protobuf? #76

Closed
justinsb opened this issue Jan 21, 2017 · 15 comments
Closed

Default to protobuf? #76

justinsb opened this issue Jan 21, 2017 · 15 comments
Labels
lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.

Comments

@justinsb
Copy link
Member

I was reviewing some code that sets cfg.ContentType = "application/vnd.kubernetes.protobuf" to opt-in to protobuf, but then I was wondering why that is needed (context
kubernetes/ingress-nginx#143 (comment))

Can client-go just default to protobuf? As a user of client-go, I don't feel like I'm in a better position to make that decision than client-go is :-)

@wojtek-t
Copy link
Member

The reason that we didn't change the default initially was that protobufs were supported for the first time in 1.3 (and this is the time when we added those cfg.ContentType = application/vnd.kubernete.protobuf line) and we wanted to client to work also with older versions of apiserver.

However, now when we have already 3 stables releases of protobufs being supported and used by our components, I think we should just change the default.

@lavalamp @caesarxuchao - for thoughts

@deads2k
Copy link
Contributor

deads2k commented Jan 23, 2017

@smarterclayton

@ericchiang
Copy link
Contributor

Per a comment on a design doc, @smarterclayton noted that

Protobuf is still quasi "internal only" - we haven't had quite enough soak time to be 100% sure we won't have to break the schema.

Is there some place tracking the concerns around committing to the current serialization format? Maybe this package can handle any breaking changes transparently down the line?

@smarterclayton
Copy link
Contributor

I'd probably prefer to continue to treat protobuf as an internal format, with the less strict guarantees we give around perfect compatibility (like we do for JSON). Use at your own risk, we reserve the right to change it in the event we severely messed up, and JSON should be good enough for anyone.

I don't want it to be the default today.

@smarterclayton
Copy link
Contributor

https://github.com/kubernetes/community/blob/master/contributors/devel/api-conventions.md#serialization-format is still in place. It was in the docs somewhere, but can't find it now.

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

Prevent issues from auto-closing with an /lifecycle frozen comment.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or @fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 22, 2017
@fejta-bot
Copy link

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or @fejta.
/lifecycle rotten
/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jan 21, 2018
@fejta-bot
Copy link

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

@lavalamp
Copy link
Member

lavalamp commented Jun 8, 2018

Given that we use proto for our storage format, I don't think we can break it. Therefore I don't see an issue with setting it to be the default.

@lavalamp lavalamp reopened this Jun 8, 2018
@lavalamp lavalamp removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Jun 8, 2018
@sttts
Copy link
Contributor

sttts commented Jun 11, 2018

If we want to default to protobuf, we probably need a tag for the client-go generator to switch to protobuf for those types that support it and leave it at json for CRDs (and other groups without proto support). Also note that we had native API groups for some time without proto support (those in staging like apiextensions.k8s.io).

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 9, 2018
@fejta-bot
Copy link

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Oct 9, 2018
@fejta-bot
Copy link

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

@k8s-ci-robot
Copy link
Contributor

@fejta-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@linxiulei
Copy link
Contributor

Any possibility we review this?

@wojtek-t

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.
Projects
None yet
Development

No branches or pull requests