My tuppence worth - agree with Thomas, it looks like a change best done in a V2. Since it's actually a real breakage for the API it would be at least worth passing through a deprecation cycle, but as Alex points out there's no efficient way to do that. The cost of restoring ugliness in the response objects at least keeps the pain away from client API consumers.
G On Wed, 8 Nov 2017 at 11:22 Graeme Miller <gra...@cloudsoft.io> wrote: > 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 > > >
