GALAglobal / TAPICC-API-implementation

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

WebHooks implementation #45

Open ysavourel opened 6 years ago

ysavourel commented 6 years ago

The webhook defined are not linked to a specific job or task or asset. So how do we know which one to call when for example a taskUpdated event occurs?

Let's say a client creates 2 jobs, with each 2 assets which have 1 tasks. that gives us 4 tasks:

Let's say we create one webhook for that client:

Let say task 211 is updated: The TAPICC server would go through the list of all webhooks for that client, find the ones for taskUpdated and fire them. Most likely there will be only one webhook for taskUpdated per client (but it's not specified anywhere).

For the client to be able to do anything meaningful either:

Also we would need to decide what kind of call is to be send: a POST, PUT or GET. Or have that information in the webhook properties.

Another question: Are the event types too generic? Would it make sense to have only types of event corresponding to changes meaningful for the client? (e.g. when a task is paused, the client may not care about it: the lone task event that most clients will be watching for is when the task is done or fails)

Alino commented 6 years ago

Webhooks are currently not implemented in this TAPICC implementation.

In general, webhooks usually are making a POST request to the webhook URL. So I would expect that create, update, delete actions on any collection should POST the whole object in the request body to the webhook URL.

The update action could also send a diff of the changes between the old object, and the updated one.

Would it make sense to have only types of event corresponding to changes meaningful for the client? (e.g. when a task is paused, the client may not care about it: the lone task event that most clients will be watching for is when the task is done or fails)

I think the webhooks should not care about what data has been updated, and it should just trigger on the specific event without complicated conditions (at least for now, but can be implemented later when needed). The checks what was updated could be handled on the client side who is receiving the webhook data.

terales commented 5 years ago

It seems that we don't need webhooks: we just need to pass TAPICC API URL and systems will be able to communicate by a predefined set of requests: image

What do you think?

Alino commented 5 years ago

I think it makes sense, I have removed webhooks for now from swagger. Closing this.

Alino commented 5 years ago

I just had a call with Jim, and I realised there could be third system which would for example want to know about when a Job was created. That would be a good use case for webhooks, so I am reopening this and putting back webhooks to swagger.