GALAglobal / TAPICC-API-implementation

TAPICC API implementation using node.js framework sails.js
Other
6 stars 1 forks source link

multiple ways to get things done #39

Closed Alino closed 6 years ago

Alino commented 6 years ago

Feedback from Jeroen:

One of the things that I would try to avoid (and the currently generated swagger file seems to have that) is to provide multiple ways to get things done, for example you can upload and download files in a lot of different ways: directly via an asset, via a task, via a job, etc, etc. This feels convenient for a consumer of the API, but it has two downsides: Every implementor of the API needs to support all those different routes and functions, making their job much harder Consumers of the API will need to choose between multiple options and will look for/ask for guidance on this, which then needs to be documented and laid down in best practices documents etc.

For an initial version of the API I would suggest to keep the number of different routes to an absolute minimum and remove everything that can be accessed or achieved in a different way as well. So provide one way to create a job, one way to create an asset and one way to create a task. I would only add multiple alternative ways if there are explicit requests from the user community: either on the implementor side or on the consumer side. That keeps the API clean, simple and easy to implement which will probably help with adoption. It also makes the intended use much clearer which is easier for API consumers.

You can always add alternative routes later that extend the API and do not break compatibility in future versions of the API.

Alino commented 6 years ago

I have realised only recently when I was implementing the file upload for Assets. That we create an Asset by uploading an asset file, but also it is possible to create Asset during Job creation. But the difference is, that during Job creation, the Asset files are empty and there is no way to upload a file into an existing Asset record, so this overcomplicates things. It would be better to have one way to create Job, Asset, Task...

ysavourel commented 6 years ago

+1 on Jeroen's comment.

Alino commented 6 years ago

I think after I have moved the swagger definition to swaggerHub, and removed few duplicates and reorganised Tags, the swagger UI documentation looks much cleaner now, without duplicates.

Can you please review version 0.0.7 and let me know what would you change in order so that we can close this issue?

https://app.swaggerhub.com/apis/Alino/tapicc-api/0.0.7#/

Thank you

ysavourel commented 6 years ago

I think it's a lot cleaner now. We may have to add/remove some commands still, but as far as the issue of duplication I think it's done.

Alino commented 6 years ago

@jeroenhuinink do you agree that we can close this issue? Or did we miss / misunderstood something?

jeroenhuinink commented 6 years ago

Yes, this is much cleaner, which makes it easier to implement (less to do) and makes it easier to use (no need to guess which path to take to get to something).