Hello, I agree with Thomas here. This seems like an API breaking change, and should be reserved for V2 if it at all. I lean towards reverting.
Regards, Graeme On 8 November 2017 at 10:01, Thomas Bouron <thomas.bou...@cloudsoftcorp.com> wrote: > Hi all. > > I'm not a fan of excluding fields from the JSON payload, if empty, for few > reasons: > 1. this is a breaking change for the UI and CLI which will be time > consuming to fix (very fiddly to guard against this in JS for example) > 2. this makes it harder to design clients, because you need to guard > against the presence of those fields. The swagger page displays all fields > leading the user/dev to think that they are always returned in the payload > 3. I don't think it saves bandwidth to remove a `constraints: []` from the > final JSON. If you want to have a smaller payload, I would much prefer to > set a query string flag to restrict (or expand) the full body, something > like `?summary=true` => returns only necessary information. > > Now, my comments apply for the API endpoints under /v1 only. I'm all in > favour of break things with a v2 API though. > > Best. > > On Mon, 6 Nov 2017 at 15:17 Alex Heneveld <alex.henev...@cloudsoftcorp.com > > > wrote: > > > > > Hi All- > > > > Until recently our REST API returned full records in most cases, > > including often lots of empty lists and maps and sometimes nulls -- such > > as `constraints: []` on all config keys. > > > > The widespread preference in REST / JSON community seems to be to omit > > these unless there is a very good reason for having them, e.g. Google > > JSON Style Guide [1]. > > > > Recently in replacing deprecated FasterXML annotations with newer ones > > many fields were changed to be included only if NON_EMPTY, in line with > > that preference, but this is technically a breaking change. And it's > > not just technical, as there are a few places in UI code which assume > > fields exist which now do break. These are easy to fix once they're > > encountered at runtime but tedious to ensure no problems at design time. > > > > Thus we have two choices: > > > > * Yes, make this break now. This (v1.0) is probably the best time. > > There is no efficient way to pass this through a deprecation cycle. Cost > > is adding to release nodes and REST API client breakages and fixes. > > * No, revert any `Include(NON_EMPTY)` annotations recently introduced to > > be strict about backwards-compatibility here. Cost is restoring old > > behaviour and bloat/ugliness in the API response objects. > > > > I have a preference for "Yes", roll-forward and tolerate some breakages > > with the shift to 1.0. > > > > Best > > Alex > > > > > > [1] > > > > https://google.github.io/styleguide/jsoncstyleguide. > xml#Empty/Null_Property_Values > > > > -- > > Thomas Bouron • Senior Software Engineer @ Cloudsoft Corporation • > https://cloudsoft.io/ > Github: https://github.com/tbouron > Twitter: https://twitter.com/eltibouron >
