camaraproject / DeviceStatus

Repository to describe, develop, document and test the Device Status API family
Apache License 2.0
11 stars 33 forks source link

Added Notification management aligned with CloudEvents format #75

Closed bigludo7 closed 8 months ago

bigludo7 commented 9 months ago

What type of PR is this?

What this PR does / why we need it:

Aligned with CloudEvents format as defined in Commonalities here: https://github.com/camaraproject/Commonalities/pull/56

Which issue(s) this PR fixes:

Fixes #74 #57

Special notes for reviewers:

Changelog input

- Provide roaming status subscription & notification aligned with Cloudevents structure.

Additional documentation

This section can be blank.

docs
akoshunyadi commented 9 months ago

@bigludo7 This PR fixes also Issue #57 ?

bigludo7 commented 9 months ago

@bigludo7 This PR fixes also Issue #57 ?

@akoshunyadi correct ! - fixed.

sachinvodafone commented 8 months ago

In some responses, we are having missing "status":

Screenshot 2023-10-25 at 12 29 30
sachinvodafone commented 8 months ago

What kind of response can the client anticipate when they receive a subscription ID in a 202 response code (asynchronous process) and attempt to access /subscriptions/{subscriptionId} where process is still not completed ?

  1. 202 Accepted (Still Processing):
  2. 404 Not Found (Subscription ID Not Found) Screenshot 2023-10-25 at 12 43 57
bigludo7 commented 8 months ago

What kind of response can the client anticipate when they receive a subscription ID in a 202 response code (asynchronous process) and attempt to access /subscriptions/{subscriptionId} where process is still not completed ?

  1. 202 Accepted (Still Processing):
  2. 404 Not Found (Subscription ID Not Found)
Screenshot 2023-10-25 at 12 43 57

Good question and probably should tackled in a larger extend than this API. I will imagine 202 is a better option.

sachinvodafone commented 8 months ago

What kind of response can the client anticipate when they receive a subscription ID in a 202 response code (asynchronous process) and attempt to access /subscriptions/{subscriptionId} where process is still not completed ?

  1. 202 Accepted (Still Processing):
  2. 404 Not Found (Subscription ID Not Found)
Screenshot 2023-10-25 at 12 43 57

Good question and probably should tackled in a larger extend than this API. I will imagine 202 is a better option.

So in this scenario, we must have 202 in response code against "/subscriptions/{subscriptionId}"

sachinvodafone commented 8 months ago

Typo mistake "0" instead of "500" error code without valid code and message in response example:

Screenshot 2023-10-27 at 11 04 07
bigludo7 commented 8 months ago

Thanks @sachinvodafone for the approval; @akoshunyadi and @fernandopradocabrillo looking also for your approval or comments. Your are not 'formal code owners but as you provided comments I'l also looking for your approval. Thanks

fernandopradocabrillo commented 8 months ago

What kind of response can the client anticipate when they receive a subscription ID in a 202 response code (asynchronous process) and attempt to access /subscriptions/{subscriptionId} where process is still not completed ?

  1. 202 Accepted (Still Processing):
  2. 404 Not Found (Subscription ID Not Found)
Screenshot 2023-10-25 at 12 43 57

Good question and probably should tackled in a larger extend than this API. I will imagine 202 is a better option.

So in this scenario, we must have 202 in response code against "/subscriptions/{subscriptionId}"

I'm not really sure a 202 is the best option to return. If I create a subscription, receive a 202 and request the info before it is created, the subscription hasn't been created yet so I'd say it is better a 404.

I haven't seen a 202 in a GET, it is a bit weird. I think it adds complexity to the implementation because the response may vary if I make the call and receive a 202 and 5mins later I receive a 200 with the info, this does not happen with the POST/PUT operations since you only make the call once. And this will force the backend to have some way of tracking the process of creating the subscription to know what to return depending on the moment.

IMO I think it is more common to return a 404 and when the subscription is created, return the 200.

cc: @bigludo7 @sachinvodafone

bigludo7 commented 8 months ago

What kind of response can the client anticipate when they receive a subscription ID in a 202 response code (asynchronous process) and attempt to access /subscriptions/{subscriptionId} where process is still not completed ?

  1. 202 Accepted (Still Processing):
  2. 404 Not Found (Subscription ID Not Found)
Screenshot 2023-10-25 at 12 43 57

Good question and probably should tackled in a larger extend than this API. I will imagine 202 is a better option.

So in this scenario, we must have 202 in response code against "/subscriptions/{subscriptionId}"

I'm not really sure a 202 is the best option to return. If I create a subscription, receive a 202 and request the info before it is created, the subscription hasn't been created yet so I'd say it is better a 404.

I haven't seen a 202 in a GET, it is a bit weird. I think it adds complexity to the implementation because the response may vary if I make the call and receive a 202 and 5mins later I receive a 200 with the info, this does not happen with the POST/PUT operations since you only make the call once. And this will force the backend to have some way of tracking the process of creating the subscription to know what to return depending on the moment.

IMO I think it is more common to return a 404 and when the subscription is created, return the 200.

cc: @bigludo7 @sachinvodafone

OK! I guess need extra discussion for adding 202 for GET when we have potentially ASYNC creation. As this topic is larger to this API I have removed the 202 and adding this point here: https://github.com/camaraproject/Commonalities/issues/83 @sachinvodafone hope it's ok for you.