camaraproject / DeviceStatus

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

feat: use simple String for "type" in CloudEvent #108

Closed maxl2287 closed 5 months ago

maxl2287 commented 8 months ago

What type of PR is this?

Add one of the following kinds:

What this PR does / why we need it:

Uses a simple string for the "type" property in CloudEvent instead of an enum.

Which issue(s) this PR fixes:

Fixes #107

bigludo7 commented 8 months ago

Hello @maxl2287 Why removing the enum for SubscriptionEventType ?

We have still it in Device Location:

    SubscriptionEventType:
      type: string
      description: |
        area-entered - Event triggered when the device enters the given area

        area-left - Event triggered when the device leaves the given area
      enum:
        - org.camaraproject.geofencing.v0.area-entered
        - org.camaraproject.geofencing.v0.area-left
maxl2287 commented 8 months ago

Hello @maxl2287 Why removing the enum for SubscriptionEventType ?

We have still it in Device Location:

    SubscriptionEventType:
      type: string
      description: |
        area-entered - Event triggered when the device enters the given area

        area-left - Event triggered when the device leaves the given area
      enum:
        - org.camaraproject.geofencing.v0.area-entered
        - org.camaraproject.geofencing.v0.area-left

Hi @bigludo7,

As it is then never used in the document anymore.

Also in DeviceLocation (geofencing), you can see that this component is not used.

maxl2287 commented 8 months ago

I see that we just use this additional SubscriptionEventType here (and NotificationEventType in geofencing) because of the "subscription-ends".

But this is causing us problems when we have a reference to an enum in CloudEvent

bigludo7 commented 8 months ago

I see that we just use this additional SubscriptionEventType here (and NotificationEventType in geofencing) because of the "subscription-ends".

But this is causing us problems when we have a reference to an enum in CloudEvent

This is used in the yaml.

I get your point about alignement with CloudEvents standard but we probably need a discussion at Commonalities level because it impacts all API with notification.

akoshunyadi commented 8 months ago

In Geofencing there is a typo type: $ref: "#/components/schemas/SubcriptionEventType"

maxl2287 commented 8 months ago

In Geofencing there is a typo type: $ref: "#/components/schemas/SubcriptionEventType"

@akoshunyadi I made a fix about that in https://github.com/camaraproject/DeviceLocation/pull/149

akoshunyadi commented 5 months ago

@maxl2287 The corresponding commonalities issue has been closed, can we close this too, or do we still need a change?

maxl2287 commented 5 months ago

Closed as the commonalities-issue is closed.