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

Server JSON responses using EmitDefaults #271

Merged
merged 1 commit into from
Sep 16, 2020
Merged

Conversation

marioizquierdo
Copy link
Contributor

Issue #103

Generated Twirp servers include default values on JSON responses by default. JSON Marshaller uses EmitDefault: true by default.

This is configurable through the server option: twirp.WithServerJSONSkipDefaults(true). If the option is set, JSON responses will not include empty values. This may be useful to save bandwidth, or to enable full backwards compatibility if the previous behavior was really to somebody.

This solution is similar to the one proposed on #134, except for the following differences:

  • The option to enable/disable this behavior is in a service constructor option, not on the code-generation command. Code generation stays simple with no flags.
  • Only the server was modified to emit defaults. Clients will behave exactly like before. This is because clients don't need to send default values; only responses need it for clarity and visibility when debugging services.
  • This PR can be included with the upcoming v7, which is a good opportunity to include a small non-backwards compatible change like this one.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@marioizquierdo marioizquierdo merged commit 0615145 into master Sep 16, 2020
@marioizquierdo marioizquierdo deleted the json_emit_defaults branch September 16, 2020 06:28
ccmtaylor added a commit to soundcloud/twinagle that referenced this pull request Nov 3, 2020
Upstream Twirp recomends this behaviour to reduce confusion.
This is fully backwards-compatible.

See-Also: twitchtv/twirp#271
ccmtaylor added a commit to soundcloud/twinagle that referenced this pull request Nov 3, 2020
Upstream Twirp recomends this behaviour to reduce confusion.
This is fully backwards-compatible.

See-Also: twitchtv/twirp#271
ccmtaylor added a commit to soundcloud/twinagle that referenced this pull request Nov 11, 2020
Upstream Twirp recomends this behaviour to reduce confusion.
This is fully backwards-compatible.

See-Also: twitchtv/twirp#271
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.

1 participant