Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Subscription & events for Commonalities 0.5.0 #313

Open
wants to merge 31 commits into
base: main
Choose a base branch
from

Conversation

bigludo7
Copy link
Collaborator

@bigludo7 bigludo7 commented Oct 10, 2024

What type of PR is this?

Add one of the following kinds:

  • documentation

What this PR does / why we need it:

This PR covers update to subscriptions & events part for Commonalities 0.5.0

Which issue(s) this PR fixes:

Fixes #

Special notes for reviewers:

Not ready for review

Changelog input

 release-note

Additional documentation

This section can be blank.

docs

Add  SUBSCRIPTION_UNPROCESSABLE in terminationReason enum for subscription ends event.
Solve camaraproject#243
Explicit subscription-end termination reason usage.
Fix camaraproject#243
Update the "sink" property description
Fix camaraproject#289
Update the "sink" property descriptionFix camaraproject#289
Add information for exiresAT + limitation to subscriptionMaxEvent and subscriptionExpireTime.
Fixes camaraproject#303
Proposal for camaraproject#276

- POST /subscriptions response will have only `id`
- removed `sinkCredentials` from GET representation
Add a note on combined usage of initialEvent and subscriptionMaxEvents (camaraproject#299)
Copy link
Collaborator

@PedroDiez PedroDiez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

18/OCT:

Fine to me so far. Editorial suggestions only

documentation/API-design-guidelines.md Outdated Show resolved Hide resolved
documentation/API-design-guidelines.md Outdated Show resolved Hide resolved
documentation/API-design-guidelines.md Outdated Show resolved Hide resolved
documentation/API-design-guidelines.md Outdated Show resolved Hide resolved
documentation/API-design-guidelines.md Outdated Show resolved Hide resolved
documentation/API-design-guidelines.md Outdated Show resolved Hide resolved
documentation/API-design-guidelines.md Outdated Show resolved Hide resolved
documentation/API-design-guidelines.md Outdated Show resolved Hide resolved
bigludo7 and others added 12 commits October 18, 2024 14:56
Co-authored-by: Pedro Díez García <pedro.diezgarcia@telefonica.com>
Co-authored-by: Pedro Díez García <pedro.diezgarcia@telefonica.com>
Co-authored-by: Pedro Díez García <pedro.diezgarcia@telefonica.com>
Co-authored-by: Pedro Díez García <pedro.diezgarcia@telefonica.com>
Co-authored-by: Pedro Díez García <pedro.diezgarcia@telefonica.com>
Co-authored-by: Pedro Díez García <pedro.diezgarcia@telefonica.com>
Co-authored-by: Pedro Díez García <pedro.diezgarcia@telefonica.com>
Co-authored-by: Pedro Díez García <pedro.diezgarcia@telefonica.com>
Co-authored-by: Pedro Díez García <pedro.diezgarcia@telefonica.com>
Highlight Subscription status value table
Add link to artifacts/camara-cloudevents/event-subscription-template.yaml
@bigludo7
Copy link
Collaborator Author

18/OCT:

Fine to me so far. Editorial suggestions only

Thanks you Pedro !
Everything considered !!

@@ -1672,18 +1694,27 @@ The rationale for using this alternate pattern should be explicitly provided
(e.g. the notification source for each of the supported events may be completely different,
in which case, separating the implementations is beneficial).

##### Rules for subscriptions data minimization
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A placeholder added as comment to consider the generic advice hint mentioned here for event structure design :) #295 (comment)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added the sentence: Application of data minimization design must be considered by the API working group for event structure definition.

bigludo7 and others added 4 commits October 22, 2024 08:41
Co-authored-by: Shilpa Padgaonkar <77152136+shilpa-padgaonkar@users.noreply.github.com>
Co-authored-by: Shilpa Padgaonkar <77152136+shilpa-padgaonkar@users.noreply.github.com>
Co-authored-by: Shilpa Padgaonkar <77152136+shilpa-padgaonkar@users.noreply.github.com>
Adding Application of data minimization design must be considered by the API working group for event structure definition.
@bigludo7
Copy link
Collaborator Author

@rartych Points to be discussed in Oct 28th call - given our planning I hope to have resolution quickly to close on this PR:

cc: @PedroDiez @shilpa-padgaonkar

@PedroDiez
Copy link
Collaborator

@rartych Points to be discussed in Oct 28th call - given our planning I hope to have resolution quickly to close on this PR:

cc: @PedroDiez @shilpa-padgaonkar

My two cents:

@rartych
Copy link
Collaborator

rartych commented Oct 28, 2024

I agree with @PedroDiez proposals
#295 needs to be resolved separately as it can cover not only subscriptions.

Remove 403_INVALID_TOKEN_CONTEXT
adding GENERIC_422_UNNECESSARY_IDENTIFIER
adding GENERIC_422_MISSING_IDENTIFIER
@bigludo7 bigludo7 changed the title (WIP) Subscription & events for Commonalities 0.5.0 Subscription & events for Commonalities 0.5.0 Oct 29, 2024
@bigludo7
Copy link
Collaborator Author

Hello team (in particular @PedroDiez @rartych @shilpa-padgaonkar )
PR ready for a first review - Thank you for your help

Copy link
Collaborator

@PedroDiez PedroDiez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor comment

LGTM so far

bigludo7 and others added 2 commits October 29, 2024 12:37
Co-authored-by: Pedro Díez García <pedro.diezgarcia@telefonica.com>
Remove error code 500 & 503
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants