andreasohlund / APIComparer

Compares NuGetPackages/Assemblies and displays changes in the public api.
MIT License
51 stars 4 forks source link

Add an API to retrieve diff results in JSON format #46

Open richorama opened 9 years ago

richorama commented 9 years ago

An API supporting diff results as JSON would be very useful.

I'm planning on using this to support version comparison on http://nugot.net

It would also be useful if CORS/JSONP was supported, allowing me to retrieve the diff directly from the browser.

andreasohlund commented 9 years ago

On thing to note is that pulling down the nugets and creating the diff can take a few seconds the first time. The current site is polling until it gets the data. Should we consider:

  1. A similar API approach where we hand back the "result" url (claim check pattern) and have the consumer call it until they get a 200 back
  2. Make the api call block until results are available
  3. Other option?

Thoughts?

cc @ParticularLabs/apicomparer

richorama commented 9 years ago

I'd be happy for it just to block until the result is complete.

danielmarbach commented 9 years ago

Blocking is so oldschool ;)

Am 24.08.2015 um 20:22 schrieb Andreas Öhlund notifications@github.com:

On think to note is that pulling down the nugets and creating the diff can take a few seconds the first time. The current site is polling until it gets the data. Should we consider:

A similar API approach where we hand back the "result" url (claim check pattern) and have the consumer call it until they get a 200 back Make the api call block until results are available Other option? Thoughts?

cc @ParticularLabs/apicomparer

— Reply to this email directly or view it on GitHub.

richorama commented 9 years ago

...another option is the client registers a webhook, which you call upon completion.

I prefer a simple blocking HTTP call, it's not like you're keeping the client waiting that long (and it's all done async using ajax anyway), everything else adds complexity, especially when you scale to multiple web servers.

andreasohlund commented 9 years ago

Agree with @richorama , lets go simple first?

danielmarbach commented 9 years ago

What defines simplicity? Is it from the implementation or usage perspective? As far as implementing having an HTTP endpoint for the command and a relocate to a query endpoint I wouldn't consider it complex implementation-wise. From a usage perspective it is, yes.

http://highscalability.com/blog/2015/8/24/ask-highscalability-choose-an-async-app-server-or-multiple-b.html

Currently not an issue but when we designed the first approach we said because downloading nugets etc can take time we would like a non-blocking way. Now when we talk about the API level we favour "simplicity" by ignoring our initial non-functional requirement?

Am 25.08.2015 um 10:21 schrieb Andreas Öhlund notifications@github.com:

Agree with @richorama , lets go simple first?

— Reply to this email directly or view it on GitHub.

richorama commented 9 years ago

I'll clarify my position. From the perspective of the consumer (I think) it's easier to have a single http get to retrieve the results, which are returned on the response to that single request.

From an implementation perspective I would suggest you use async & await, and don't block a thread, but that's an implementation detail (or optimisation).

This has the advantage that http responses are essentially immutable, and can be heavily cached.

If this isn't achievable/desirable, then I think the next best option is to use one http post to start the comparison process, the client can then make multiple gets to query the progress, and attempt to retrieve the result.

andreasohlund commented 9 years ago

I do see your point @danielmarbach

How about we do this.

Case 1 - Results already available

Since we cache the generated diffs we can just send them back right away with a 200 if possible

Case 2 - No comparison available

If the packageid, leftversion, rightversion combination haven't been diffed before we (sort of what we already do for th html page)

  1. Send of a internal "request" to create the comparison
  2. Return a 202 to the the requestor that we got it
  3. Keep on returning 202 until the comparison is available
  4. Return 200 and the content as soon as available

This will force the client to keep on calling until a 200 is returned

nulltoken commented 9 years ago

:+1: Although returning 202 for a GET reads a bit weird, the "come back later" approach makes a lot a sense.

One thing that may also be added on top of it would be headers preventing any kind of caching for 202 responses, and a different one for 200 which would allow a long lasting caching at any downstream level of the response (provided, of course, the comparison has been successful. Wouldn't that be the case, an explicit error message should be returned as part of the json payload static the issue. One last thought, it may make sense to prevent caching on those error responses as the source of the issue may eventually be resolved (eg. cannot compare because of missing package with this version and, later, the package is uploaded...))

andreasohlund commented 9 years ago

Agreed, good points regarding the caching.

Side note: we actually render the results as html and stores it so its cheap to return them again. We would do the same for json responses I imagine.

On Fri, Sep 4, 2015 at 7:25 PM, nulltoken notifications@github.com wrote:

[image: :+1:] Although returning 202 for a GET, the "come back later" approach makes a lot a sense.

One thing that may also be added on top of it would be headers preventing any kind of caching for 202 responses, and a different one for 200 which would allow a long lasting caching at any downstream level of the response (provided, of course, the comparison has been successful. Wouldn't that be the case, an explicit error message should be returned as part of the json payload static the issue. One last thought, it may make sense to prevent caching on those error responses as the source of the issue may eventually be resolved (eg. cannot compare because of missing package with this version and, later, the package is uploaded...))

— Reply to this email directly or view it on GitHub https://github.com/ParticularLabs/APIComparer/issues/46#issuecomment-137798287 .