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

Ensure forward compatibility of experimental enums #467

Closed
joffrey-bion opened this issue Dec 7, 2024 · 4 comments
Closed

Ensure forward compatibility of experimental enums #467

joffrey-bion opened this issue Dec 7, 2024 · 4 comments
Labels
enhancement New feature or request

Comments

@joffrey-bion
Copy link
Owner

joffrey-bion commented Dec 7, 2024

As we have seen in #466, sometime the tip-of-tree protocol definitions don't contain the most recent values of some enums from experimental APIs.

To prevent deserialization crashes in these cases, we should somehow handle unknown values without crashing.
There are several options to go about it:

  1. provide a synthetic UNDEFINED value for such enums, and use it when deserializing values that are not defined in the protocol
    • pros: simple, not much disruption when the enum goes stable
    • cons: the original JSON value is lost after deserialization
  2. turn these enums into sealed classes with objects for each known value, and a data class Unknown(val value: String) for the unknown ones.
    • pros: lets the consumer know about the exact unknown value that was encountered
    • cons: big source incompatibility change when an enum goes stable, much more complicated to implement

We could also mitigate the transition to stable by applying this solution to all enums, but this puts some hassle on consumers of stable APIs, which probably will never change.

Open question: what should be considered an experimental enum? A lot of enums are not marked as experimental themselves, but can be used in experimental types or APIs. We would probably have to check whether they are used exclusively in experimental types/APIs, and consider them as expeimental themselves in this case.

@joffrey-bion
Copy link
Owner Author

Another option along the lines of option 2 above: https://github.com/bright/codified

@joffrey-bion
Copy link
Owner Author

joffrey-bion commented Dec 7, 2024

As a first step, this will be implemented with option 1. We'll see if there is a need for more complicated approaches later.

@pcarrier
Copy link

pcarrier commented Dec 7, 2024

My 2c: for #466 I would typically want to distinguish notRendered from uninteresting when processing the axtree response.

@joffrey-bion
Copy link
Owner Author

joffrey-bion commented Dec 9, 2024

Ah! I had the solution 1 almost ready to ship this weekend. But I hesitated quite a bit because of the mentioned drawbacks. Ok since you show interest in distinguishing unknown values. I'll think of another approach. Either a variant of these CodifiedEnums, or maybe a simple value class with some constants.

This will also solve my naming dilemma for option 1's unknown value. The values unknown and undefined are already valid values in some of the enums of the protocol!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants