No json pretty printing #313

Closed
wants to merge 1 commit into
from

Projects

None yet

3 participants

@sebastianmika

Changes proposed in this pull request:

  • Do not pretty print returned json payload

Personally I think it is not best practice to return pretty printed json. This only unnecessarily enlarges the payload to be returned and this increase can be significant. Pretty printing should be
handled by the client instead.

@sebastianmika sebastianmika Fix: Personally I think it is not best practice to return pretty printed
json. This only unnecessarily enlarges the payload to be
returned and this increase can be significant. Pretty printing should be
handled by the client instead
665b25f
@sebastianmika

As a side note: I can of course return a flask Response object with my own non-pretty printed json - but then response validation in connexion does not work, and this is one of the feature I like (amongst others) very much.

@coveralls
coveralls commented Oct 11, 2016 edited

Coverage Status

Coverage remained the same at 100.0% when pulling 665b25f on sebastianmika:master into 5892feb on zalando:master.

@hjacobs
Member
hjacobs commented Oct 12, 2016

@sebastianmika I personally agree, the response JSON should be machine-readable, humans can use their favorite client (e.g. HTTPie) to get pretty output.

@hjacobs
Member
hjacobs commented Oct 12, 2016

@sebastianmika basically this PR reverts #193, see the discussion there (BTW I still agree with you about not having pretty printed by default).

@sebastianmika

True enough.

Regarding what is best practice or not, I found little relevant stuff. However, at one place it was correctly stated that enabling compression is much more important than whether one pretty prints the json or not.

Using flask-compress and adding to my create_app

compress = Compress()
compress.init_app(app.app)

that is quickly done and indeed had a much greater impact than not pretty printing.

The same source also argued as gdb from #193 - API results should be nice to look at for developers without any hassle.

So, I would rather close my own PR and go with compression, unless you make a strong point against that.

As an alternative, as also discussed in #193, pretty or not could be an option to connexion.App and/or add_api, but I don't know when I would have the time to add that.

@hjacobs
Member
hjacobs commented Oct 12, 2016

@sebastianmika OK, if you are fine with closing your PR 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment