search

openphacts / GLOBAL

Global project issues [private for now. owner lee harland]
3 stars 0 forks source link

Update swagger to 2.0 #318

Closed antonisloizou closed 8 years ago

antonisloizou commented 8 years ago

Try to update to 1.2 Swagger version , but at least make it parse for 2.0

antonisloizou commented 8 years ago

So I just updated the develop docs. There were quite a few changes needed to upgrade, but it should now parse for Evan.

We can apply custom CSS styles to try to get it to look more like the old version, but I like this presentation better. What do you think @nicklynch , @danidi ?

The more major issue is that we can no longer have several APIs on the same path , as was the case with the InCHi, InCHiKey and SMILES to URL methods (/structure). They have now been merged into a single Swagger API description, but the underlying methods stay exactly the same. That is , no changes are required to clients for this.

While we decide what to do here, I've kept both versions, swagger-1.0.json and swagger-2.0.json

nicklynch commented 8 years ago

I really like the new interface and its easy to browse.

So we can test with Knime etc, is the actual Swagger the same? http://ops2.few.vu.nl/api-config-files/swagger.json

I reckon Updating to new Swagger is something we would need to do at some point anyway and this is a good time as any to make this change? Thoughts @antonisloizou @danidi

Would the potential new methods be: /structure/ConvertInchitoURI /structure/ConvertSMILEStoURI etc?

Had a quick look at the stats for these methods in 1.4 and 1.5 over the last month and 6 months

For 1.5, its averaging 40-60 hits a day but had a single day peak of 600 this month For 1.5 had a peak in July of 30k in a day For 1.4, its averaging 10-20 hits a day but had a single day peak of 1000

danidi commented 8 years ago

It will take some time to get used to it, but I think it is good to have all API calls around one concept blocked together, as the list was already very long. Still, I quite liked the colour coded blocks we had before ;) And when changing to API 2.0, I guess we can also change the look. It's great that you can now copy/paste parts of the response easily.

For the structure methods, I wouldn't split them into different ones. As they are basically doing the same thing, I don't think we would need seperate calls. The only problem, as far as I see it is that you can't just run the example query, as it pastes in all three options, leading to a 400 error. Maybe in this case, having the example just in the description would be necessary.

And just a detail: Mapping service betweem URLs should be between. Also, the description column could be a bit wider (especially as parameter type and data type don't need that much space).

antonisloizou commented 8 years ago

@nicklynch :

antonisloizou commented 8 years ago

@danidi :

antonisloizou commented 8 years ago

@nicklynch , @danidi

As a final note on the new version is that we can now add response templates in JSON schema. See http://editor.swagger.io/#/ for an example.

This will also be very time consuming to do across our 78 calls, so I would propose postponing to at least 2.1

danidi commented 8 years ago

As none of the Structure search parameters is required, I would leave all empty (but have the examples in the description).

nicklynch commented 8 years ago

@antonisloizou @danidi Agree on having search parameters just in the examples so we can keep the API the same [Chemical Structure to URI] Would suggest we leave them all blank except for Smiles?, and add text to description such as 'Please enter one query term in either Smiles, Inchi or Inchi_key field below'

nicklynch commented 8 years ago

Now fixed in new release