Repository navigation
Fix documentation issues - #91
Conversation
Fix documentation issues
CAMARA Validation — PASS0 errors, 0 warnings, 16 hints | Profile: standard |
There was a problem hiding this comment.
In order to avoid working twice (i.e. to define specific error response schemas now), we are fine if this PR is scoped and merged in these terms, adding Generic 429 Error case.
Nonetheless, our view is that scenarios applicability are:
IdentifierNotFound404 - https://github.com/camaraproject/Commonalities/blob/r4.4/artifacts/common/CAMARA_common.yaml#L517
TooManyRequests429 - https://github.com/camaraproject/Commonalities/blob/r4.4/artifacts/common/CAMARA_common.yaml#L624
That align can be done later after triggering RC cycle referred to Commonalities r4.4
| $ref: '#/components/responses/Generic404' | ||
| "422": | ||
| $ref: '#/components/responses/Generic422' | ||
| "429": |
There was a problem hiding this comment.
Think 429 case is only TOO_MANY_REQUESTS
We are fine if this topic is addressed later, after triggering RC cycle, based on Commonalities r4.4
TooManyRequests429 - https://github.com/camaraproject/Commonalities/blob/r4.4/artifacts/common/CAMARA_common.yaml#L624
There was a problem hiding this comment.
@PedroDiez , I've included Common artifacts - Generic429 to be consistent with the others errors blocs. But it's ok to limit to 429-TOO_MANY_REQUESTS.
@ToshiWakayama-KDDI, WDYT ?
There was a problem hiding this comment.
It is fine to use Generic429 now and set up the limitation to 429-TOO_MANY_REQUESTS, when doing alignment with Commonalities r4.4
There was a problem hiding this comment.
Thanks, @GillesInnov35 , @PedroDiez .
Sorry, I don't uderstand this part fully. Generic429 is now only for 429-TOO_MANY_REQUEST, based on the Commonalities r4.4, correct?
But Line716 and after include GENERIC_429_QUOTA_EXCEEDED as well. I don't understand this. Or GENERIC_429_QUOTA_EXCEEDED has to be removed but not yet done?
BR
Toshi
There was a problem hiding this comment.
What I mean @ToshiWakayama-KDDI is that the applicable 429 Error in this API is only 429-TOO_MANY_REQUEST.
Generic429 provides both TOO_MANY_REQUEST and QUOTA_EXCEEDED.
It is fine in this PR to keep as is, but when doing Aligment with Commonalities r4.4 in a future PR this error model in 429 scenario should be replaced by:
TooManyRequests429 - https://github.com/camaraproject/Commonalities/blob/r4.4/artifacts/common/CAMARA_common.yaml#L624
Hope this helps for the understading
| $ref: '#/components/responses/Generic401' | ||
| "403": | ||
| $ref: '#/components/responses/Generic403' | ||
| "404": |
There was a problem hiding this comment.
Think only applies IdentifierNotFound404 - https://github.com/camaraproject/Commonalities/blob/r4.4/artifacts/common/CAMARA_common.yaml#L517
We are fine if this topic is addressed later, after triggering RC cycle, based on Commonalities r4.4
PedroDiez
left a comment
There was a problem hiding this comment.
Fine to proceed now as it is and later with Commonalities r4.4 aligment make fine grained alignment.
IdentifierNotFound404 - https://github.com/camaraproject/Commonalities/blob/r4.4/artifacts/common/CAMARA_common.yaml#L517
TooManyRequests429 - https://github.com/camaraproject/Commonalities/blob/r4.4/artifacts/common/CAMARA_common.yaml#L624
@GillesInnov35, @ToshiWakayama-KDDI if you are ok
Left LGTM in advance
|
thanks @PedroDiez , I'm OK with your proposal but I'm wondering if we remove NOT_FOUND from HTTP 404 error code enum it'll not be a non backward compatibility with V0.4.0. |
Hi @GillesInnov35, We can do safely as we are going to generate a major version v1, so it can be handled in a compatible way by API Consumers |
|
@ToshiWakayama-KDDI , @fernandopradocabrillo , @albertoramosmonagas , concerning issues below
I thought we've added somewhere a link to jaro-winkler definition but not found. Does CAMARA recommends any web site, Wikipedia or any others ? We could simply move algorithm reference higher up. I'd prefer not to modify the sentence.
@PedroDiez (Telefonica) proposes to remove 404 - Not Found error (Commonalities 4.4). If approved by code owners, this issue will closed. WDYT ? |
From TEF side:
...Unless otherwise captured in the implementation notes which Operators may publish, score will use the That link is a good simplification as it summarizes mathematical model and mention the canonical references (1) and (2).
|
PedroDiez
left a comment
There was a problem hiding this comment.
LGTM
Please @GillesInnov35, @ToshiWakayama-KDDI, are you ok with doing this in a later PR when doing Commonalities r4.4 aligment?
We would like to confirm from codeowners that IdentifierNotFound404 - https://github.com/camaraproject/Commonalities/blob/r4.4/artifacts/common/CAMARA_common.yaml#L517 is the applicable 404 case. It is not needed to cover it within this PR. Just to have track of it to apply when making Commonalities r4.4 alignment related PR.
|
|
@ToshiWakayama-KDDI , @fernandopradocabrillo , could you review this PR. @ToshiWakayama-KDDI , I think question about 404 - NOT_FOUND error removed in error section (IdentifierNotFound404 would be the applicable 404 case) might have an impact on your side. Thanks |
| * **KYC**: stands for Know Your Customer and it is the process of a business verifying the identity of their clients and assessing their suitability, along with the potential risks of illegal intentions towards the business relationship. | ||
|
|
||
| * **Match Score**: a numerical value that quantifies the similarity between two pieces of text based on the words they contain. This score is often used in various applications like text comparison, plagiarism detection, information retrieval, and natural language processing. The score typically reflects how well the words in one text match the words in another text. In the context of this API, this score will be used to determine how much does the input information looks like the information stored in the Operator's system. Unless otherwise captured in the implementation notes which Operators may publish, score will use the Jaro-Winkler distance algorithm for all countries. This parameter, as optional, will be returned depending on the capability of the Operator to calculate the scoring value. This means that not all Operators will implement this functionality or won't have the requested parameter available. It can happen that an Operator implements the score functionality but, for whatever reason, is not able to calculate it based on the client's input or the related stored information. For these cases, the score property related won't be returned in the response. | ||
| * **Match Score**: a numerical value that quantifies the similarity between two pieces of text based on the words they contain. This score is often used in various applications like text comparison, plagiarism detection, information retrieval, and natural language processing. The score typically reflects how well the words in one text match the words in another text. In the context of this API, this score will be used to determine how much does the input information looks like the information stored in the Operator's system. Unless otherwise captured in the implementation notes which Operators may publish, score will use the [Jaro-Winkler](https://en.wikipedia.org/wiki/Jaro%E2%80%93Winkler_distance) distance algorithm for all countries. This parameter, as optional, will be returned depending on the capability of the Operator to calculate the scoring value. This means that not all Operators will implement this functionality or won't have the requested parameter available. It can happen that an Operator implements the score functionality but, for whatever reason, is not able to calculate it based on the client's input or the related stored information. For these cases, the score property related won't be returned in the response. |
There was a problem hiding this comment.
Thanks, @GillesInnov35 .
To make this requirement clearer, I would like to suggest making this part bold, like Unless otherwise captured ~~~ algorithm for all countries.
WDTY?
| $ref: '#/components/schemas/KycMatchRequestBody' | ||
| examples: | ||
| Two-Legged and Three-leg Access Token Example: | ||
| Two-Legged Example: |
There was a problem hiding this comment.
Thanks, @GillesInnov35 .
To improve consistency with the descriptions in the earlier sections of the YAML (where we use the terms "using a Two-legged access token" and "using a Three-legged access token"), I would like to suggest updating the example titles as follows:
- Rename "Two-Legged and Three-leg Access Token Example" to "Example when using a Two-legged access token"
- Rename "Three-Legged Without Phone Number Example" to "Example when using a Three-legged access token"
This alignment will make the specification much clearer and more intuitive for API consumers. What do you think?
BR
Toshi
What type of PR is this?
Add one of the following kinds:
What this PR does / why we need it:
Fix documentation issues
Which issue(s) this PR fixes:
Fixes #87