Skip to content

Update geofencing-subscriptions for Spring25#284

Merged
maxl2287 merged 5 commits intocamaraproject:mainfrom
maxl2287:feat/error-alignement-with-commonalities
Jan 15, 2025
Merged

Update geofencing-subscriptions for Spring25#284
maxl2287 merged 5 commits intocamaraproject:mainfrom
maxl2287:feat/error-alignement-with-commonalities

Conversation

@maxl2287
Copy link
Contributor

@maxl2287 maxl2287 commented Jan 6, 2025 •
edited
Loading

What type of PR is this?

  • enhancement/feature

What this PR does / why we need it:

Updates location-verification API with the changes derived from Commonalities and ICM updates
x-camara-guidelines: 0.5.0

Which issue(s) this PR fixes:

Fixes #276

Special notes for reviewers:

There are still some open discussions in Commonalities that may impact this PR, so we will keep it open until there is a final version for x-camara-guidelines: 0.5.0

Changelog input

* Error schemas updated with enums
* Some error status and codes have been updated
* Add a note that initial events will be counted when subscriptionMaxEvents is combined with initialEvent=true

@maxl2287 maxl2287 added enhancement New feature or request Spring25 Meta-release Spring25 labels Jan 6, 2025
@maxl2287 maxl2287 self-assigned this Jan 6, 2025
bigludo7
bigludo7 reviewed Jan 7, 2025
View reviewed changes
Copy link
Collaborator

@bigludo7 bigludo7 left a comment

Choose a reason for hiding this comment

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

Thanks @maxl2287

Few comments - mot of them related to the PR361 of commonalities (which is not yet merged but hope soon to be):

  • 429 is not required for GET & DELETE (as described here).
  • It probably misses an example for 401: AUTHENTICATION_REQUIRED - look the PR361
  • For 422 we have to align the code value with the PR361 also - like IDENTIFIER_MISMATCH instead of DEVICE_IDENTIFIERS_MISMATCH, SERVICE_NOT_APPLICABLE instead of DEVICE_NOT_APPLICABLE
  • I guess the 422 UNNECESSARY_IDENTIFIER, MISSING_IDENTIFIER & UNSUPPORTED_IDENTIFIER examples must be added.

@maxl2287 maxl2287 mentioned this pull request Jan 8, 2025
Closed
@maxl2287 maxl2287 requested a review from bigludo7 January 8, 2025 09:42
bigludo7
bigludo7 approved these changes Jan 8, 2025
View reviewed changes
Copy link
Collaborator

@bigludo7 bigludo7 left a comment

Choose a reason for hiding this comment

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

Thanks Max

LGTM

bigludo7
bigludo7 previously approved these changes Jan 8, 2025
View reviewed changes
Copy link
Collaborator

@bigludo7 bigludo7 left a comment

Choose a reason for hiding this comment

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

LGTM

@maxl2287 maxl2287 force-pushed the feat/error-alignement-with-commonalities branch from aa577ef to 5a7c7de Compare January 13, 2025 13:51
@maxl2287
Copy link
Contributor Author

maxl2287 commented Jan 13, 2025

Sorry I mistakingly pushed here a commit, what I wanted to do in a seperate branch.
No updates here done.
Please re-approve @bigludo7

@maxl2287 maxl2287 requested a review from bigludo7 January 13, 2025 13:55
bigludo7
bigludo7 approved these changes Jan 13, 2025
View reviewed changes
Copy link
Collaborator

@bigludo7 bigludo7 left a comment

Choose a reason for hiding this comment

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

sure !

jlurien
jlurien approved these changes Jan 14, 2025
View reviewed changes
Copy link
Collaborator

@jlurien jlurien left a comment

Choose a reason for hiding this comment

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

LGTM so far
Do we need to update the test plan as well? Please double check that error codes are still valid

@maxl2287
Copy link
Contributor Author

maxl2287 commented Jan 14, 2025

LGTM so far Do we need to update the test plan as well? Please double check that error codes are still valid

@jlurien I will create a seperate PR after we have merged this one here, where I will add addtional test cases for HTTP-422.
These are not yet existing.

@maxl2287 maxl2287 mentioned this pull request Jan 15, 2025
Merged
@maxl2287 maxl2287 merged commit e50ff4e into camaraproject:main Jan 15, 2025
2 checks passed
@maxl2287 maxl2287 deleted the feat/error-alignement-with-commonalities branch January 15, 2025 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jlurien jlurien jlurien approved these changes

@bigludo7 bigludo7 bigludo7 approved these changes

Assignees

@maxl2287 maxl2287

Labels

enhancement New feature or request Spring25 Meta-release Spring25

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Error Alignement with Commonalities

3 participants

@maxl2287 @jlurien @bigludo7