Create subscription-notification-template.yaml #189
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Create subscription-notification-template.yaml aligned with CloudEvents subscription model. Refer to camaraproject#185
bigludo7
requested review from
shilpa-padgaonkar,
rartych,
patrice-conil,
RubenBG7 and
PedroDiez
as code owners
maxl2287
reviewed
Apr 19, 2024
artifacts/camara-cloudevents/subscription-notification-template.yaml
Outdated
Show resolved
Hide resolved
artifacts/camara-cloudevents/subscription-notification-template.yaml
Outdated
Show resolved
Hide resolved
artifacts/camara-cloudevents/subscription-notification-template.yaml
Outdated
Show resolved
Hide resolved
Udpaded following Maximilian review
akoshunyadi
reviewed
Apr 19, 2024
artifacts/camara-cloudevents/subscription-notification-template.yaml
Outdated
Show resolved
Hide resolved
artifacts/camara-cloudevents/subscription-notification-template.yaml
Outdated
Show resolved
Hide resolved
| description: The detail of the requested event subscription | ||
| type: object | ||
| required: | ||
| - type |
There was a problem hiding this comment.
Is this type the same as types on the top level? That is an array...
Then actually we would need an array of CreateSubscriptionDetail too, the different types can have different parameters
There was a problem hiding this comment.
@akoshunyadi I think I did a mistake there. I need to remove type from CreateSubscriptionDetail as the type will be at a top level --> I will fix.
For the array of CreateSubscriptionDetail I will be cautious because we
- decide to be very cautious about really managing multi-events subscription: not for the first meta release, and then to be discussed for future release at API level
- we can have multi event subscription only when event-type are very close so assuming they will have same subscription detail (the good exemple is subscribing to geofencing-device-entered & geofencing-device-left with same area monitored & same
So for my perspective we can keep CreateSubscriptionDetail as it @akoshunyadi @shilpa-padgaonkar @PedroDiez WDYT?
rartych
reviewed
Apr 19, 2024
artifacts/camara-cloudevents/subscription-notification-template.yaml
Outdated
Show resolved
Hide resolved
rartych
reviewed
Apr 19, 2024
artifacts/camara-cloudevents/subscription-notification-template.yaml
Outdated
Show resolved
Hide resolved
rartych
reviewed
Apr 19, 2024
artifacts/camara-cloudevents/subscription-notification-template.yaml
Outdated
Show resolved
Hide resolved
rartych
reviewed
Apr 19, 2024
artifacts/camara-cloudevents/subscription-notification-template.yaml
Outdated
Show resolved
Hide resolved
…e.yaml Co-authored-by: Rafal Artych <121048129+rartych@users.noreply.github.com>
…e.yaml Co-authored-by: Rafal Artych <121048129+rartych@users.noreply.github.com>
…template.yaml As suggested by @akoshunyadi
Rename xxx to event-type Fixed issue with security remove example for apiVersion
bigludo7
commented
Apr 22, 2024
Fixed issue for scope in the openId security part (line 40)
bigludo7
commented
Apr 22, 2024
|
@bigludo7 : Looks like some copy paste error. Could you kindly check L757? |
|
Because I worked on this with @bigludo7, I'll let you approve or not this proposal |
Co-authored-by: Rafal Artych <121048129+rartych@users.noreply.github.com>
rartych
reviewed
May 28, 2024
Co-authored-by: Rafal Artych <121048129+rartych@users.noreply.github.com>
rartych
reviewed
May 28, 2024
Co-authored-by: Rafal Artych <121048129+rartych@users.noreply.github.com>
rartych
reviewed
May 28, 2024
Co-authored-by: Rafal Artych <121048129+rartych@users.noreply.github.com>
rartych
reviewed
May 28, 2024
rartych
reviewed
May 28, 2024
rartych
reviewed
May 28, 2024
Co-authored-by: Rafal Artych <121048129+rartych@users.noreply.github.com>
Co-authored-by: Rafal Artych <121048129+rartych@users.noreply.github.com>
Co-authored-by: Rafal Artych <121048129+rartych@users.noreply.github.com>
rartych
reviewed
May 28, 2024
Co-authored-by: Rafal Artych <121048129+rartych@users.noreply.github.com>
|
Thanks a lot @bigludo7 for this PR and @PedroDiez for your detailed review! |
bigludo7
dismissed stale reviews from shilpa-padgaonkar and PedroDiez
via
07b6ad1
Co-authored-by: Pedro Díez García <pedro.diezgarcia@telefonica.com>
|
Thanks a lot @PedroDiez & @shilpa-padgaonkar for your help - great team work. I've committed last minor editorial from Pedro so this dismissed your approval Shilpa. Once you'll both approved, I guess we can merge this @rartych ? |
|
LGTM |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Create subscription-notification-template.yaml aligned with CloudEvents subscription model. Refer to #185
What type of PR is this?
Add one of the following kinds:
What this PR does / why we need it:
Create subscription-notification-template.yaml aligned with CloudEvents subscription model.
Refer to #185
Which issue(s) this PR fixes:
Fixes #185
Special notes for reviewers:
Changelog input
Additional documentation
This section can be blank.