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.

Sign up for GitHub

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

Jump to bottom

Update device error model #232

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

Update device error model #232

wants to merge 7 commits into from

Conversation

fernandopradocabrillo
Copy link
Collaborator

What type of PR is this?

Add one of the following kinds:

  • correction
  • enhancement/feature
  • documentation

What this PR does / why we need it:

Align error model with Commonalities for meta Spring25

Which issue(s) this PR fixes:

Fixes #230

Special notes for reviewers:

In the subscriptions APIs there is one error that I'm not sure if we have to update or not.

TokenMismatch:
   value:
    status: 403
    code: SUBSCRIPTION_MISMATCH
    message: Inconsistent access token for requested events subscription

It looks like it is the equivalent to 403 INVALID_TOKEN_CONTEXT. It may make sense to keep it since the device object is required when creating the subscription, regardless of the token type. I tend to think we should keep it.

@bigludo7
Copy link
Collaborator

bigludo7 commented Dec 6, 2024

@fernandopradocabrillo Agree with you to keep it but in order to be explicit the code should not be: INVALID_TOKEN_CONTEXT ?

@bigludo7
Copy link
Collaborator

bigludo7 commented Dec 8, 2024

Hi @fernandopradocabrillo
Additional thinking:

  • The callback must have 429 in device-roaming-status-subscriptions.yaml (in lines 140/155)

Following points are open:

  • Do we keep 500, 501, 502, 503 & 504 in the yamls ? they are not anymore mandatory to be defined
  • 429 is not mandatory for the GET /subscriptions/{subscriptionId} & GET /subscriptions.
  • same for DELETE

@fernandopradocabrillo
Copy link
Collaborator Author

fernandopradocabrillo commented Dec 10, 2024 •
edited
Loading

@fernandopradocabrillo Agree with you to keep it but in order to be explicit the code should not be: INVALID_TOKEN_CONTEXT ?

I'm not sure, I see that in geofencing it is also SUBSCRIPTION_MISMATCH: https://github.com/camaraproject/DeviceLocation/blob/main/code/API_definitions/geofencing-subscriptions.yaml#L1166

do you know if there is any conversation right now in commonalities regarding the errors in subscriptions?

@akoshunyadi
Copy link
Collaborator

@fernandopradocabrillo would it be possible to cover the 2 new APIs here as well? 🙂 (connected-network-type-subscriptions is still to be merged)

@akoshunyadi akoshunyadi mentioned this pull request Dec 18, 2024
Open
@fernandopradocabrillo
Copy link
Collaborator Author

fernandopradocabrillo commented Dec 19, 2024 •
edited
Loading

@fernandopradocabrillo would it be possible to cover the 2 new APIs here as well? 🙂 (connected-network-type-subscriptions is still to be merged)

yeah and I won't charge extra

@fernandopradocabrillo
Copy link
Collaborator Author

@camaraproject/device-status_maintainers shouldn't the POST /subscription endpoint also have the 404 IDENTIFIER_NOT_FOUND error?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eric-murray eric-murray Awaiting requested review from eric-murray eric-murray is a code owner

@bigludo7 bigludo7 Awaiting requested review from bigludo7 bigludo7 is a code owner

@sachinvodafone sachinvodafone Awaiting requested review from sachinvodafone sachinvodafone is a code owner

@akoshunyadi akoshunyadi Awaiting requested review from akoshunyadi akoshunyadi is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
Spring25
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Align Device Errors with last Commonalities agreement for Spring25
3 participants
@fernandopradocabrillo @bigludo7 @akoshunyadi