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

Marshal protobuf enums as strings #377

Open
marcogschmidt opened this issue Sep 1, 2020 · 3 comments
Open

Marshal protobuf enums as strings #377

marcogschmidt opened this issue Sep 1, 2020 · 3 comments
Assignees

Comments

@marcogschmidt
Copy link
Collaborator

No description provided.

@marcogschmidt marcogschmidt self-assigned this Sep 1, 2020
@yuval-k
Copy link
Member

yuval-k commented Sep 1, 2020

sounds like a breaking change..?

@marcogschmidt
Copy link
Collaborator Author

@yuval-k I think the change would not be breaking, as we unmarshal resources using the jsonpb.Unmarshal function, which can handle enums both when they have been serialized are strings and as ints.

I had created this issue as I thought that is was a prerequisite for a Gloo change, but it turns out it is not, so I don't think I will implement it right now. It would be good to do it at some point though, because currently the state of a resource is serialized as an int, which is not the best UX (e.g. a user will see state: 1, instead of state: Accepted). If we set EnumsAsInts to false on the marshaler that we use for the resource status, we would fix this without breaking backwards compatibility.

@yuval-k
Copy link
Member

yuval-k commented Sep 2, 2020

i meant for our users; the CRD shape is now different; if someone monitors it, expecting to see state: 1 then it might break them..
we can engage our users to see if that's a problem for them; i agree with you that the UX will be better with this change

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

No branches or pull requests

2 participants