camaraproject / DeviceLocation

Repository to describe, develop, document and test the DeviceLocation API family
Apache License 2.0
21 stars 32 forks source link

Geofencing - Adding `SUBSCRIPTION_DELETED` as a Termination Reason #141

Closed bigludo7 closed 2 months ago

bigludo7 commented 8 months ago

Problem description Feedback from implementation. For the terminationReason we have following value:

We did not have a specific value when the termination was triggered from a DELETE operation.

We can assume that because the consume side perform the DELETE no need for notification but for sake of consistency send an event seems to be a good option.

Possible evolution Add value SUBSCRIPTION_DELETED in the terminationReason array

Alternative solution

Additional context

jlurien commented 8 months ago

This may have sense, but it is something that would apply to subscriptions for all APIs, wdyt @PedroDiez ?

alpaycetin74 commented 8 months ago

If we assume deletion of subscription is a synchronous operation, successful response to DELETE operation is sufficient. If we believe the deletion of subscription in the backend must be done asynchronously, a notification is necessary because successful response to DELETE would only mean the backend received & accepted the request.

To be flexible, we can support both. But then we should define 2 different success responses to DEL so that the client would now if deletion is completed or they should wait for a notification.

Having said that, we could also make creation of subscription asynchronous (HTTP 201 created followed by a notification later). But we never considered it. Maybe it is also not necessary for DEL ?

bigludo7 commented 8 months ago

Hello @alpaycetin74 For me the question is not about the fact that the DELETE is performed async or sync. As we send a 204 the deletion is sync for the system triggering the DELETE.

But we cannot assume technical correlation between the system listening to the event channel (the target of the notificationUrl) and the system performing the DELETE (OK this is the problem of the consumer). But as we are committed to send a suscription-ends notification to the listener, the point for me is to provide information about subscription termination reason to this listener.

Does it make sense?

alpaycetin74 commented 8 months ago

hello @bigludo7 , yes, especially when thinking of _"we cannot assume technical correlation between the system listening to the event channel (the target of the notificationUrl) and the system performing the DELETE" it makes sense. It could help the listener application to shut down, maybe save some resources/cost/battery.

PedroDiez commented 8 months ago

This may have sense, but it is something that would apply to subscriptions for all APIs, wdyt @PedroDiez ?

Yes, think makes sense for APIs that have explicit subscriptions model (i.e. Resource-based). Think this is something to be indidated/align in commonalities

bigludo7 commented 7 months ago

Hello Perhaps we can move forward on this as same event type hase been added in QoD API & Device Status API (https://github.com/camaraproject/DeviceStatus/issues/117).

To be aligned with this API we should use DELETE_REQUESTED wording.

alpaycetin74 commented 7 months ago

I have seen @akoshunyadi 's comment saying "DELETE_REQUESTED" sounds like it is not final, and still some final event is to come. I'd prefer SUBSCRIPTION_DELETED as well.

bigludo7 commented 7 months ago

Hello @alpaycetin74 ....yes agreed with you of the interpretation but probably the idea is to have consistence with QoD API. (https://github.com/camaraproject/QualityOnDemand/blob/v0.10.0/code/API_definitions/qod-api.yaml#L918).

Conclusion: make sense to set this at commonalities level to have a de facto alignement.

akoshunyadi commented 6 months ago

The discussion about subscription lifecycle management in commonalities: https://github.com/camaraproject/Commonalities/issues/153