-
Notifications
You must be signed in to change notification settings - Fork 219
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
Add support for custom default values #1286
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JordonPhillips
requested changes
Jun 27, 2022
mtdowling
force-pushed
the
default-custom-value
branch
from
June 28, 2022 22:22
9341dea
to
807e310
Compare
JordonPhillips
approved these changes
Jun 30, 2022
kstich
requested changes
Jun 30, 2022
smithy-model/src/main/java/software/amazon/smithy/model/loader/IdlModelParser.java
Show resolved
Hide resolved
smithy-model/src/main/java/software/amazon/smithy/model/shapes/ShapeType.java
Outdated
Show resolved
Hide resolved
smithy-model/src/test/java/software/amazon/smithy/model/knowledge/NullableIndexTest.java
Outdated
Show resolved
Hide resolved
docs/source/1.0/spec/core/traits/type-refinement-traits.rst.template
Outdated
Show resolved
Hide resolved
mtdowling
force-pushed
the
default-custom-value
branch
from
June 30, 2022 22:07
807e310
to
5485700
Compare
kstich
approved these changes
Jun 30, 2022
This commit updates Smithy IDL v2 to support custom default values rather than only default zero values. We didn't previously support custom default values because we wanted to be able to change default values if ever needed, we wanted to avoid information disclosure of unreleased internal members, and we wanted clients to be able to fill in a default zero value if a required member is missing on the wire (implying that the member transitioned from required to default). While default zero values are a big simplification for clients, they do come with awkward tradeoffs. For example, the zero value of an enum was `""` (or it could have been the first enum in the shape definition but that's problematic due to model filtering, and it requires a lot of up-front planning from service teams). We also observed that there are likely 1,000+ members already used in AWS that have default values that are only captured through documentation. If we could actually model defaults, it would be easier to catch changes to default values in automated diff tools and discuss them in API reviews. To address these concerns and support custom default values, we recommend the following: * clients implement presence tracking and only serialize default values if the member is explicitly set to the default value. This allows services to change defaults if ever necessary. * servers always serialize default values unless the member is marked with the `@internal` trait. This makes it explicit as to what the server thinks the default value is while still preventing unintended information disclosure of unreleased features. * To avoid downtime and account for misconfigured servers that do not serialize required or default members, clients attempt to populate the member with a default zero value. More details and specifics can be found in defaults-and-model-evolution.md. Enum changes: * Given the addition of syntax sugar for assigning values to defaulted structure members, this commit also allows syntactic sugar for assigning values to enum and intEnum shapes without the use of the `@enumValue` trait. * Now that there is no default zero value for enums, the `@enumDefault` trait has been removed.
mtdowling
force-pushed
the
default-custom-value
branch
from
June 30, 2022 22:42
5485700
to
9598892
Compare
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit updates Smithy IDL v2 to support custom default values
rather than only default zero values.
We didn't previoulsy support custom default values because we wanted to
be able to change default values if ever needed, we wanted to avoid
information disclosure of unreleased internal members, and we wanted
clients to be able to fill in a default zero value if a required member
is missing on the wire (implying that the member transitioned from
required to default).
While default zero values are a big simplification for clients, they do
come with awkward tradeoffs. For example, the zero value of an enum was
""
(or it could have been the first enum in the shape definition butthat's problematic due to model filtering, and it requires a lot of
up-front planning from service teams). We also observed that there are
likely 1,000+ members already used in AWS that have default values that
are only captured through documentation. If we could actually model
defaults, it would be easier to catch changes to default values in
automated diff tools and discuss them in API reviews.
To address these concerns and support custom default values, we
recommend the following:
the member is explicitly set to the default value. This allows services
to change defaults if ever necessary.
the
@internal
trait. This makes it explicit as to what the serverthinks the default value is while still preventing unintended
information disclosure of unreleased features.
serialize required or default members, clients attempt to populate the
member with a default zero value.
More details and specifics can be found in
defaults-and-model-evolution.md.
Enum changes:
defaulted structure members, this commit also allows syntactic sugar for
assigning values to enum and intEnum shapes without the use of the
@enumValue
trait.@enumDefault
trait has been removed.
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.