Add fuzzy logic for pretty detection

[tyll]

References: #1

[pypingou]

Honestly, I do not like this, mdapi is really not meant for human consumption and I think it should be made explicit when we want it, otherwise, just be as fast as possible.

Note: mdapi is hosted on pagure, not on github so we should move there (I'm waiting for #1 to be closed to block issue-creation on github, I forgot to do it before).

[techtonik]

@tyll awesome, 👍 just what I meant.

@pypingou I think you're biased. People choose JSON because it is human readable. Otherwise there are things like http://msgpack.org/index.html Also, what is "as fast as possible"? Do you have any data?

I support the guy who said:

We should forget about small efficiencies, say about 97% of the time: premature optimization is the root of all evil."

Thanks to Wikipedia that contains the exact argument that I could not extract from the depths of my neural network:

Optimization can reduce readability and add code that is used only to improve the performance. This may complicate programs or systems, making them harder to maintain and debug.

Non-human readable format make is harder to maintain and debug any app which is built upon your service. And reading once more through that - I would drop ?pretty=True entirely making it default behaviour.

https://en.wikipedia.org/wiki/Program_optimization#When_to_optimize

[pypingou]

I support the guy who said:

 We should forget about small efficiencies, say about 97% of the time:
 premature optimization is the root of all evil."

Thanks to Wikipedia that contains the exact argument that I could not
extract from the depths of my neural network:

 Optimization can reduce readability and add code that is used only to
 improve the performance. This may complicate programs or systems, making
 them harder to maintain and debug.

In the present case, making the output human-readable adds more code and makes
it more complicated (see this PR, it adds code, doesn't remove any).

Non-human readable format make is harder to maintain and debug any app
which is built upon your service. And reading once more through that - I
would drop ?pretty=True entirely making it default behaviour.

And I am currently still not convinced.

[techtonik]

Strange, but #1 is 404.

@pypingou my opinion is that you can't be convinced. No offence, it just the facts - first I see someone from open source world closing commenting without any alternative means of feedback. It also seems to me that someone likes to reinvent things from scratch. For example why didn't you fork https://kallithea-scm.org/ ? Nothing wrong about it - we sometimes rewrite this lib or another, but the two facts combined lead me to conclusion that I may deal with a person who is not comfortable with other opinions or accepting them. For example, you did an excellent job making performance analysis, but you don't want to redo this for this change.

In the present case, making the output human-readable adds more code and makes it more complicated (see this PR, it adds code, doesn't remove any).

It was not about you - for a person who single-handedly coded pagure.io - common, I don't believe that those two lines made the code more complicated. And I said - make ?pretty=True just normal behaviour and you don't need that option. It was about application maintainers, who will base their applications on your service, and when your output changes, server config changes, service goes offline, they have to debug their scripts. And when they add ?pretty=True, the output will become different.

P.S. I also curious why do you host your projects on GitHub at all? Your pagure project there doesn't have any feedback mechanism except private email. This one doesn't have even that. If it is a way to lure GitHub users into your platform then make it at least easy with GitHub SSO.

[pypingou]

Strange, but #1 is 404.

Yes, I did say I was turning off issues on github, I prefer to have the tickets in one place (and I had forgot to disable the ticketing system on github).

closing commenting without any alternative means of feedback.

Pagure has tickets just like github: https://pagure.io/mdapi/issues

[hguemar]

My 2cts: the change itself does not change anything for people using the raw API, but it makes debugging for humans slightly easier by displaying prettily JSON when using a browser.
About the optimization question, I'm not sure it's an issue but you can easily merge the 2 ifs (see my inline comment)

[ralphbean]

Am I the only one who uses JSONovich?

[ralphbean]

Sorry, that comment was kind of out of line. It is actually about including pretty-printing of JSON as a feature at all -- and not as much about this automatic detection.

We have examples of other apps that do different things based on a mimetype check/guess. Bodhi does it. Datagrepper does it. So, we have precedent for that.

• Do any of our other APIs do automatic JSON prettification like this?
• If we accept this, does that mean we need to add it to all our other APIs to be consistent?
• Just to re-iterate, it seems like we're adding server-side code here that could be nicely addressed by using client-side tools (JSONovich for firefox or $ http --pretty for the console).

[hguemar]

@ralphbean didn't know about JSONovitch, nice tool.
I think we should accept this PR, and yes, this behaviour should be consistent all accross the API (maybe using a decorator?)

[ralphbean]

Yeah, if we were to accept this and copy it everywhere, a decorator would be the way to go. FWIW, all our JSON encoding should probably go through something like that anyways to make a change like this easy. However, that is currently not the case with many of our flask apps.

That aside, I'm pretty lukewarm on this PR. It just seems like extra code on the server to do what client side tools do very nicely. Earlier in the thread, @techtonik suggested that people choose JSON because it is human readable. I think a stronger argument for JSON is that it is easily managed by javascript on web frontends (that's why I like it anyways). It is relatively readable but not necessarily so. It is analogous to other language-natives types like python dicts or ruby hashes which is nice because the programmer doesn't have to perform mental gymnastics to convert between things.

So, 😐. @tyll, as the original author, do you want to add to the discussion here?

[tyll]

On Tue, Nov 24, 2015 at 06:57:01AM -0800, Ralph Bean wrote: So, :neutral_face:. @tyll, as the original author, do you want to add to the discussion here?

I agree that developers usually also use dedicated tools to look at JSON APIs, I do it as well if necessary. However I would also enjoy it more to fiddle around with a new API, when I can just take a look at it. E.g. when I first saw https://apps.fedoraproject.org/mdapi with example links to https://apps.fedoraproject.org/mdapi/rawhide/files/kernel-core I would have been more impressed if it was directly pretty-printed and I did not have to open a terminal to look at it properly. Obviously I think of it as good feature that I bothered creating a pull request, but I will also not fight it to the death.

[relrod]

I don't think JSON is used for human-readability. That said, the index page of mdapi has actual links that people can click on to demo the functionality. Since links are typically clicked on from a browser, I think it is worth making those link to something pretty when we can trivially do that.

So in my opinion, this comes down to:

• Do we match on content-type as in this PR?
• Or do we change those links to point to ?pretty=1?

I would be +1 on either of those changes.

I don't see any huge disadvantage to the PR here, and it keeps the links on the index page true to how the API will typically be called by a client (i.e., without ?pretty=1). So I have a (very) slight preference toward merging this PR in.

The PR seems simple enough and I haven't seen anything objective against it. "It is more code" doesn't really persuade me either way. If number of characters of code is a concern, we could start using single-letter variables everywhere ;). I kid, of course, but I hope it makes my point.

[pypingou]

Ok, so I tested this code and it doesn't work because the request object has no accept_mimetypes.

I'm going to close this PR and open another one fixing this.

[pypingou]

New PR is up for review at: https://pagure.io/mdapi/pull-request/22

[techtonik]

E.g. when I first saw

https://apps.fedoraproject.org/mdapi

with example links to

https://apps.fedoraproject.org/mdapi/rawhide/files/kernel-core

I would have been more impressed if it was directly pretty-printed and I
did not have to open a terminal to look at it properly.

That's exactly my thoughts about it. More than that - I will probably use this more often than other services with obtrusive web interfaces. Chrome remember links that I once typed and going to address bar to enter query is just one keyboard action, and going through the dedicated site and clicking forms is too complicated compared to that.

Obviously I think of it as good feature that I bothered creating a pull request, but I will also not fight it to the death.

I will. =)

I think a stronger argument for JSON is that it is easily managed by javascript on web frontends (that's why I like it anyways).

Well, yes. I'd say there are not much (any?) alternatives for structured data on the web that can be used without loading a bunch of parser code.

Thanks everybody for consideration of this feature and to @pypingou for this awesome service.

[tyll]

Thank you. Do you know which kind of object request is? I assumed it was from python-werkzeug.

[pypingou]

@tyll mdapi is written in aiohttp which has its own request objects