Add Face API v1.2-preview.1 #29781
Merged
Add Face API v1.2-preview.1 #29781
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
Next Steps to Merge✅ All automated merging requirements have been met! To get your PR merged, see aka.ms/azsdk/specreview/merge. |
Swagger Validation Report
|
| Compared specs (v0.10.12) | new version | base version |
|---|---|---|
| Face.json | v1.1-preview.1(b71d136) | v1.1-preview.1(main) |
| Rule | Message |
|---|---|
|
The new version is missing a definition that was found in the old version. Was 'CreateLivenessSessionContent' removed or renamed? New: Face/preview/v1.1-preview.1/Face.json#L7265:3 Old: Face/preview/v1.1-preview.1/Face.json#L7265:3 |
|
The '$ref' property points to different models in the old and new versions. New: Face/preview/v1.1-preview.1/Face.json#L413:13 Old: Face/preview/v1.1-preview.1/Face.json#L413:13 |
|
The '$ref' property points to different models in the old and new versions. New: Face/preview/v1.1-preview.1/Face.json#L5860:13 Old: Face/preview/v1.1-preview.1/Face.json#L5860:13 |
|
The new version is adding enum value(s) '2023-12-20-preview.06, 2023-12-20-preview.07, 2023-12-20-preview.08' from the old version. New: Face/preview/v1.1-preview.1/Face.json#L8751:9 Old: Face/preview/v1.1-preview.1/Face.json#L8703:9 |
|
The new version is adding enum value(s) '2023-12-20-preview.06, 2023-12-20-preview.07, 2023-12-20-preview.08' from the old version. New: Face/preview/v1.1-preview.1/Face.json#L8639:5 Old: Face/preview/v1.1-preview.1/Face.json#L8606:5 |
️❌Breaking Change(Cross-Version): 308 Errors, 257 Warnings failed [Detail]
| Compared specs (v0.10.12) | new version | base version |
|---|---|---|
| Face.json | v1.2-preview.1(b71d136) | v1.0(main) |
| Face.json | v1.2-preview.1(b71d136) | v1.1-preview.1(main) |
The following breaking changes are detected by comparison with the latest stable version:
Only 26 items are listed, please refer to log for more details.
| Rule | Message |
|---|---|
|
The new version is missing a path that was found in the old version. Was path '/snapshots' removed or restructured? Old: Face/stable/v1.0/Face.json#L2253:5 |
|
The new version is missing a path that was found in the old version. Was path '/snapshots/{snapshotId}' removed or restructured? Old: Face/stable/v1.0/Face.json#L2347:5 |
|
The new version is missing a path that was found in the old version. Was path '/snapshots/{snapshotId}/apply' removed or restructured? Old: Face/stable/v1.0/Face.json#L2449:5 |
|
The new version is missing a path that was found in the old version. Was path '/verify?overload=person' removed or restructured? Old: Face/stable/v1.0/Face.json#L2533:5 |
|
The new version is missing a path that was found in the old version. Was path '/detect?overload=stream' removed or restructured? Old: Face/stable/v1.0/Face.json#L2576:5 |
|
The new version is missing a path that was found in the old version. Was path '/persongroups/{personGroupId}/persons/{personId}/persistedfaces?overload=stream' removed or restructured? Old: Face/stable/v1.0/Face.json#L2633:5 |
|
The new version is missing a path that was found in the old version. Was path '/facelists/{faceListId}/persistedfaces?overload=stream' removed or restructured? Old: Face/stable/v1.0/Face.json#L2684:5 |
|
The new version is missing a path that was found in the old version. Was path '/largepersongroups/{largePersonGroupId}/persons/{personId}/persistedfaces?overload=stream' removed or restructured? Old: Face/stable/v1.0/Face.json#L2732:5 |
|
The new version is missing a path that was found in the old version. Was path '/largefacelists/{largeFaceListId}/persistedfaces?overload=stream' removed or restructured? Old: Face/stable/v1.0/Face.json#L2783:5 |
|
The new version is missing a definition that was found in the old version. Was 'FindSimilarRequest' removed or renamed? New: Face/preview/v1.2-preview.1/Face.json#L7055:3 Old: Face/stable/v1.0/Face.json#L2832:3 |
|
The new version is missing a definition that was found in the old version. Was 'GroupRequest' removed or renamed? New: Face/preview/v1.2-preview.1/Face.json#L7055:3 Old: Face/stable/v1.0/Face.json#L2832:3 |
|
The new version is missing a definition that was found in the old version. Was 'IdentifyRequest' removed or renamed? New: Face/preview/v1.2-preview.1/Face.json#L7055:3 Old: Face/stable/v1.0/Face.json#L2832:3 |
|
The new version is missing a definition that was found in the old version. Was 'VerifyFaceToPersonRequest' removed or renamed? New: Face/preview/v1.2-preview.1/Face.json#L7055:3 Old: Face/stable/v1.0/Face.json#L2832:3 |
|
The new version is missing a definition that was found in the old version. Was 'VerifyFaceToFaceRequest' removed or renamed? New: Face/preview/v1.2-preview.1/Face.json#L7055:3 Old: Face/stable/v1.0/Face.json#L2832:3 |
|
The new version is missing a definition that was found in the old version. Was 'UpdateFaceRequest' removed or renamed? New: Face/preview/v1.2-preview.1/Face.json#L7055:3 Old: Face/stable/v1.0/Face.json#L2832:3 |
|
The new version is missing a definition that was found in the old version. Was 'Confidence' removed or renamed? New: Face/preview/v1.2-preview.1/Face.json#L7055:3 Old: Face/stable/v1.0/Face.json#L2832:3 |
|
The new version is missing a definition that was found in the old version. Was 'NameAndUserDataContract' removed or renamed? New: Face/preview/v1.2-preview.1/Face.json#L7055:3 Old: Face/stable/v1.0/Face.json#L2832:3 |
|
The new version is missing a definition that was found in the old version. Was 'MetaDataContract' removed or renamed? New: Face/preview/v1.2-preview.1/Face.json#L7055:3 Old: Face/stable/v1.0/Face.json#L2832:3 |
|
The new version is missing a definition that was found in the old version. Was 'ApplyScope' removed or renamed? New: Face/preview/v1.2-preview.1/Face.json#L7055:3 Old: Face/stable/v1.0/Face.json#L2832:3 |
|
The new version is missing a definition that was found in the old version. Was 'ApplySnapshotRequest' removed or renamed? New: Face/preview/v1.2-preview.1/Face.json#L7055:3 Old: Face/stable/v1.0/Face.json#L2832:3 |
|
The new version is missing a definition that was found in the old version. Was 'TakeSnapshotRequest' removed or renamed? New: Face/preview/v1.2-preview.1/Face.json#L7055:3 Old: Face/stable/v1.0/Face.json#L2832:3 |
|
The new version is missing a definition that was found in the old version. Was 'UpdateSnapshotRequest' removed or renamed? New: Face/preview/v1.2-preview.1/Face.json#L7055:3 Old: Face/stable/v1.0/Face.json#L2832:3 |
|
The new version is missing a definition that was found in the old version. Was 'ImageUrl' removed or renamed? New: Face/preview/v1.2-preview.1/Face.json#L7055:3 Old: Face/stable/v1.0/Face.json#L2832:3 |
|
The operation id has been changed from 'Face_FindSimilar' to 'FaceRecognitionOperations_FindSimilar'. This will impact generated code. New: Face/preview/v1.2-preview.1/Face.json#L1535:7 Old: Face/stable/v1.0/Face.json#L31:7 |
|
The operation id has been changed from 'Face_Group' to 'FaceRecognitionOperations_Group'. This will impact generated code. New: Face/preview/v1.2-preview.1/Face.json#L1626:7 Old: Face/stable/v1.0/Face.json#L74:7 |
|
The operation id has been changed from 'Face_Identify' to 'FaceRecognitionOperations_IdentifyFromPersonGroup'. This will impact generated code. New: Face/preview/v1.2-preview.1/Face.json#L1675:7 Old: Face/stable/v1.0/Face.json#L117:7 |
The following breaking changes are detected by comparison with the latest preview version:
| Rule | Message |
|---|---|
| The new version is missing a definition that was found in the old version. Was 'CreateLivenessSessionContent' removed or renamed? New: Face/preview/v1.2-preview.1/Face.json#L7055:3 Old: Face/preview/v1.1-preview.1/Face.json#L7265:3 |
|
| The '$ref' property points to different models in the old and new versions. New: Face/preview/v1.2-preview.1/Face.json#L403:13 Old: Face/preview/v1.1-preview.1/Face.json#L413:13 |
|
| The '$ref' property points to different models in the old and new versions. New: Face/preview/v1.2-preview.1/Face.json#L5730:13 Old: Face/preview/v1.1-preview.1/Face.json#L5860:13 |
|
| The new version is adding a path that was not found in the old version. New: Face/preview/v1.2-preview.1/Face.json#L5119:5 |
|
| The new version is adding a path that was not found in the old version. New: Face/preview/v1.2-preview.1/Face.json#L5463:5 |
️️✔️CredScan succeeded [Detail] [Expand]
There is no credential detected.
️⚠️LintDiff: 119 Warnings warning [Detail]
| Compared specs (v2.2.2) | new version | base version |
|---|---|---|
| v1.1-preview.1 | v1.1-preview.1(b71d136) | v1.1-preview.1(main) |
| v1.2-preview.1 | v1.2-preview.1(b71d136) | default(main) |
[must fix]The following errors/warnings are introduced by current PR:
Only 26 items are listed, please refer to log for more details.
| Rule | Message | Related RPC [For API reviewers] |
|---|---|---|
| Booleans properties are not descriptive in all cases and can make them to use, evaluate whether is makes sense to keep the property as boolean or turn it into an enum. Location: Face/preview/v1.1-preview.1/Face.json#L7451 |
||
| Booleans properties are not descriptive in all cases and can make them to use, evaluate whether is makes sense to keep the property as boolean or turn it into an enum. Location: Face/preview/v1.1-preview.1/Face.json#L7455 |
||
| Booleans properties are not descriptive in all cases and can make them to use, evaluate whether is makes sense to keep the property as boolean or turn it into an enum. Location: Face/preview/v1.1-preview.1/Face.json#L7521 |
||
| Booleans properties are not descriptive in all cases and can make them to use, evaluate whether is makes sense to keep the property as boolean or turn it into an enum. Location: Face/preview/v1.1-preview.1/Face.json#L7525 |
||
| Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations. Location: Face/preview/v1.2-preview.1/Face.json#L88 |
||
| Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations. Location: Face/preview/v1.2-preview.1/Face.json#L345 |
||
| Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations. Location: Face/preview/v1.2-preview.1/Face.json#L393 |
||
| Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations. Location: Face/preview/v1.2-preview.1/Face.json#L431 |
||
| Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations. Location: Face/preview/v1.2-preview.1/Face.json#L464 |
||
| Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations. Location: Face/preview/v1.2-preview.1/Face.json#L497 |
||
| Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations. Location: Face/preview/v1.2-preview.1/Face.json#L553 |
||
| Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations. Location: Face/preview/v1.2-preview.1/Face.json#L601 |
||
| Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations. Location: Face/preview/v1.2-preview.1/Face.json#L647 |
||
| Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations. Location: Face/preview/v1.2-preview.1/Face.json#L680 |
||
| Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations. Location: Face/preview/v1.2-preview.1/Face.json#L713 |
||
| Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations. Location: Face/preview/v1.2-preview.1/Face.json#L769 |
||
| Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations. Location: Face/preview/v1.2-preview.1/Face.json#L820 |
||
| Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations. Location: Face/preview/v1.2-preview.1/Face.json#L857 |
||
| Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations. Location: Face/preview/v1.2-preview.1/Face.json#L932 |
||
| Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations. Location: Face/preview/v1.2-preview.1/Face.json#L1009 |
||
| Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations. Location: Face/preview/v1.2-preview.1/Face.json#L1053 |
||
| Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations. Location: Face/preview/v1.2-preview.1/Face.json#L1110 |
||
| Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations. Location: Face/preview/v1.2-preview.1/Face.json#L1150 |
||
| Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations. Location: Face/preview/v1.2-preview.1/Face.json#L1194 |
||
| Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations. Location: Face/preview/v1.2-preview.1/Face.json#L1289 |
||
| Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations. Location: Face/preview/v1.2-preview.1/Face.json#L1343 |
The following errors/warnings exist before current PR submission:
Only 26 items are listed, please refer to log for more details.
| Rule | Message |
|---|---|
| API version should be a date in YYYY-MM-DD format, optionally suffixed with '-preview'. Location: Face/preview/v1.1-preview.1/Face.json#L5 |
|
| Operation does not define an 'api-version' query parameter. Location: Face/preview/v1.1-preview.1/Face.json#L92 |
|
| Booleans properties are not descriptive in all cases and can make them to use, evaluate whether is makes sense to keep the property as boolean or turn it into an enum. Location: Face/preview/v1.1-preview.1/Face.json#L167 |
|
| Booleans properties are not descriptive in all cases and can make them to use, evaluate whether is makes sense to keep the property as boolean or turn it into an enum. Location: Face/preview/v1.1-preview.1/Face.json#L272 |
|
| Booleans properties are not descriptive in all cases and can make them to use, evaluate whether is makes sense to keep the property as boolean or turn it into an enum. Location: Face/preview/v1.1-preview.1/Face.json#L280 |
|
| Parameter should have a description. Location: Face/preview/v1.1-preview.1/Face.json#L299 |
|
| Operation does not define an 'api-version' query parameter. Location: Face/preview/v1.1-preview.1/Face.json#L354 |
|
| Operation does not define an 'api-version' query parameter. Location: Face/preview/v1.1-preview.1/Face.json#L407 |
|
| Operation does not define an 'api-version' query parameter. Location: Face/preview/v1.1-preview.1/Face.json#L449 |
|
| Path parameter should specify a maximum length (maxLength) and characters allowed (pattern). Location: Face/preview/v1.1-preview.1/Face.json#L450 |
|
| Operation does not define an 'api-version' query parameter. Location: Face/preview/v1.1-preview.1/Face.json#L488 |
|
| Path parameter should specify a maximum length (maxLength) and characters allowed (pattern). Location: Face/preview/v1.1-preview.1/Face.json#L489 |
|
A delete operation should have a 204 response.Location: Face/preview/v1.1-preview.1/Face.json#L497 |
|
| All success responses except 202 & 204 should define a response body. Location: Face/preview/v1.1-preview.1/Face.json#L498 |
|
| Operation does not define an 'api-version' query parameter. Location: Face/preview/v1.1-preview.1/Face.json#L525 |
|
| Path parameter should specify a maximum length (maxLength) and characters allowed (pattern). Location: Face/preview/v1.1-preview.1/Face.json#L526 |
|
| Operation does not define an 'api-version' query parameter. Location: Face/preview/v1.1-preview.1/Face.json#L587 |
|
| Operation does not define an 'api-version' query parameter. Location: Face/preview/v1.1-preview.1/Face.json#L643 |
|
| Check for appropriate use of formData parameters. Location: Face/preview/v1.1-preview.1/Face.json#L644 |
|
| Check for appropriate use of formData parameters. Location: Face/preview/v1.1-preview.1/Face.json#L651 |
|
| Operation does not define an 'api-version' query parameter. Location: Face/preview/v1.1-preview.1/Face.json#L690 |
|
| Path parameter should specify a maximum length (maxLength) and characters allowed (pattern). Location: Face/preview/v1.1-preview.1/Face.json#L691 |
|
| Operation does not define an 'api-version' query parameter. Location: Face/preview/v1.1-preview.1/Face.json#L729 |
|
| Path parameter should specify a maximum length (maxLength) and characters allowed (pattern). Location: Face/preview/v1.1-preview.1/Face.json#L730 |
|
A delete operation should have a 204 response.Location: Face/preview/v1.1-preview.1/Face.json#L738 |
|
| All success responses except 202 & 204 should define a response body. Location: Face/preview/v1.1-preview.1/Face.json#L739 |
️️✔️Avocado succeeded [Detail] [Expand]
Validation passes for Avocado.
️️✔️SwaggerAPIView succeeded [Detail] [Expand]
️️✔️TypeSpecAPIView succeeded [Detail] [Expand]
️❌ModelValidation: 103 Errors, 0 Warnings failed [Detail]
Only 26 items are listed, please refer to log for more details.
| Rule | Message |
|---|---|
|
x-ms-example not found in FaceDetectionOperations_DetectFromUrl. Url: Face/preview/v1.2-preview.1/Face.json#L88:15 |
|
x-ms-example not found in LivenessSessionOperations_GetLivenessSessions. Url: Face/preview/v1.2-preview.1/Face.json#L345:14 |
|
x-ms-example not found in LivenessSessionOperations_CreateLivenessSession. Url: Face/preview/v1.2-preview.1/Face.json#L393:15 |
|
x-ms-example not found in LivenessSessionOperations_GetLivenessSessionResult. Url: Face/preview/v1.2-preview.1/Face.json#L431:14 |
|
x-ms-example not found in LivenessSessionOperations_DeleteLivenessSession. Url: Face/preview/v1.2-preview.1/Face.json#L464:17 |
|
x-ms-example not found in LivenessSessionOperations_GetLivenessSessionAuditEntries. Url: Face/preview/v1.2-preview.1/Face.json#L497:14 |
|
x-ms-example not found in LivenessSessionOperations_GetLivenessWithVerifySessions. Url: Face/preview/v1.2-preview.1/Face.json#L553:14 |
|
x-ms-example not found in LivenessSessionOperations_CreateLivenessWithVerifySessionWithVerifyImage. Url: Face/preview/v1.2-preview.1/Face.json#L601:15 |
|
x-ms-example not found in LivenessSessionOperations_GetLivenessWithVerifySessionResult. Url: Face/preview/v1.2-preview.1/Face.json#L647:14 |
|
x-ms-example not found in LivenessSessionOperations_DeleteLivenessWithVerifySession. Url: Face/preview/v1.2-preview.1/Face.json#L680:17 |
|
x-ms-example not found in LivenessSessionOperations_GetLivenessWithVerifySessionAuditEntries. Url: Face/preview/v1.2-preview.1/Face.json#L713:14 |
|
x-ms-example not found in PersonDirectoryOperations_GetDynamicPersonGroups. Url: Face/preview/v1.2-preview.1/Face.json#L769:14 |
|
x-ms-example not found in PersonDirectoryOperations_GetDynamicPersonGroup. Url: Face/preview/v1.2-preview.1/Face.json#L820:14 |
|
x-ms-example not found in PersonDirectoryOperations_CreateDynamicPersonGroupWithPerson. Url: Face/preview/v1.2-preview.1/Face.json#L857:14 |
|
x-ms-example not found in PersonDirectoryOperations_UpdateDynamicPersonGroupWithPersonChanges. Url: Face/preview/v1.2-preview.1/Face.json#L932:16 |
|
x-ms-example not found in PersonDirectoryOperations_DeleteDynamicPersonGroup. Url: Face/preview/v1.2-preview.1/Face.json#L1009:17 |
|
x-ms-example not found in PersonDirectoryOperations_GetDynamicPersonGroupPersons. Url: Face/preview/v1.2-preview.1/Face.json#L1053:14 |
|
x-ms-example not found in FaceListOperations_GetFaceLists. Url: Face/preview/v1.2-preview.1/Face.json#L1110:14 |
|
x-ms-example not found in FaceListOperations_GetFaceList. Url: Face/preview/v1.2-preview.1/Face.json#L1150:14 |
|
x-ms-example not found in FaceListOperations_CreateFaceList. Url: Face/preview/v1.2-preview.1/Face.json#L1194:14 |
|
x-ms-example not found in FaceListOperations_UpdateFaceList. Url: Face/preview/v1.2-preview.1/Face.json#L1289:16 |
|
x-ms-example not found in FaceListOperations_DeleteFaceList. Url: Face/preview/v1.2-preview.1/Face.json#L1343:17 |
|
x-ms-example not found in FaceListOperations_AddFaceListFaceFromUrl. Url: Face/preview/v1.2-preview.1/Face.json#L1378:15 |
|
x-ms-example not found in FaceListOperations_DeleteFaceListFace. Url: Face/preview/v1.2-preview.1/Face.json#L1491:17 |
|
x-ms-example not found in FaceRecognitionOperations_FindSimilar. Url: Face/preview/v1.2-preview.1/Face.json#L1535:15 |
|
x-ms-example not found in FaceRecognitionOperations_Group. Url: Face/preview/v1.2-preview.1/Face.json#L1626:15 |
️️✔️SemanticValidation succeeded [Detail] [Expand]
Validation passes for SemanticValidation.
️️✔️PoliCheck succeeded [Detail] [Expand]
Validation passed for PoliCheck.
️️✔️SpellCheck succeeded [Detail] [Expand]
Validation passes for SpellCheck.
️️✔️Lint(RPaaS) succeeded [Detail] [Expand]
Validation passes for Lint(RPaaS).
️️✔️PR Summary succeeded [Detail] [Expand]
Validation passes for Summary.
️️✔️Automated merging requirements met succeeded [Detail] [Expand]
Swagger Generation Artifacts
|
Generated ApiView
|
AzureRestAPISpecReview
added
BreakingChangeReviewRequired
This was referenced Jul 10, 2024
Han-msft
requested review from
tg-msft and
weidongxu-microsoft
and removed request for
a team
|
Breaking changes previously reviewed and approved in #27576 |
mikekistler
added
the
BreakingChange-Approved-Previously
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
Comment on lines
269
to
297
| model CreateLivenessWithVerifySessionContent { | ||
| model CreateLivenessWithVerifySessionMultipartBody { |
There was a problem hiding this comment.
This appears to be an SDK break.
Java CI fails due to this (likely a difference of name in generated code and customization code).
https://github.com/Azure/azure-rest-api-specs/pull/29781/checks?check_run_id=27836423122
There was a problem hiding this comment.
It is necessary? AFAIK, the model renaming cannot be modelled via tsp versioning. Hence it appears in v1.1-preview.1 Swagger as well.
If it is for SDK, a @clientName in client.tsp is simpler.
There was a problem hiding this comment.
It's intended change to differentiate the input model.
I'm open to other solutions, but it's a little complicated.
In v1.1, the operations look like
createLivenessSession(CreateLivenessSessionContent)
createLivenessWithVerifySession(CreateLivenessSessionContent)
createLivenessWithVerifySessionWithVerifyImage(CreateLivenessWithVerifySessionContent)
model CreateLivenessWithVerifySessionContent
{
Parameters: CreateLivenessSessionContent;
VerifyImage: bytes;
}
In v1.2, liveness with verify session introduced some specific field, so it can not share the model with liveness session. CreateLivenessWithVerifySessionContent is also occupied by multipart request, that's why I renamed all of them to avoid confusion.
Please also note that CreateLivenessWithVerifySessionRequest is an additional layer added by SDK folks after our previous release, I'm not sure what's the purpose.
createLivenessSession(CreateLivenessSessionParameters)
createLivenessWithVerifySession(CreateLivenessWithVerifySessionParameters)
createLivenessWithVerifySessionWithVerifyImage(CreateLivenessWithVerifySessionRequest)
model CreateLivenessWithVerifySessionMultipartBody
{
Parameters: CreateLivenessWithVerifySessionParameters;
VerifyImage: bytes;
}
There was a problem hiding this comment.
The CreateLivenessWithVerifySessionRequest was due to https://github.com/Azure/azure-rest-api-specs/pull/29723/files for the spread thing (without it and the @body under it, the model CreateLivenessWithVerifySessionContent would be spread in the createLivenessWithVerifySessionWithVerifyImage API).
This CreateLivenessWithVerifySessionRequest should not result to a real model/class in SDK. The intention is mostly only to add that @body.
There was a problem hiding this comment.
I see. It is CreateLivenessSessionContent now renamed to CreateLivenessSessionParameters (and 2 added properties).
And createLivenessWithVerifySession/createLivenessWithVerifySessionWithVerifyImage API now need a different model, hence CreateLivenessWithVerifySessionParameters (and further 2 more properties).
The versioning of these models is hard, when rename and add is mixed together (e.g. CreateLivenessWithVerifySessionParameters is added in 1.2, but it cannot be annotated as such, since it now being used in API exists in 1.1). I don't have a good recommendation either. As both version is preview (and no REST API change in 1.1), maybe we can just make the break.
Maybe just announce it in SDK thread and see if any dev having issue with this.
There was a problem hiding this comment.
I'm thinking about another option. Redefine the CreateLivenessWithVerifySessionContent model.
createLivenessSession(CreateLivenessSessionContent)
createLivenessWithVerifySession(CreateLivenessWithVerifySessionContent)
createLivenessWithVerifySessionWithVerifyImage(CreateLivenessWithVerifySessionMultipartContent)
In this way we can avoid breaking changes on createLivenessSession since it's model does not change.
However, the breaking changes on CreateLivenessWithVerifySessionContent would looks weird since it was used for multipart in v1.1.
On the other hand, CreateLivenessWithVerifySessionContent was not exposed in .NET/Java since we had a layered of customization to merge createLivenessWithVerifySession and createLivenessWithVerifySessionWithVerifyImage.
This way would lower the breaking change scope than current implementation.
Let me discuss it with SDK developers in our team.
Comment on lines
269
to
297
| model CreateLivenessWithVerifySessionContent { | ||
| model CreateLivenessWithVerifySessionMultipartBody { |
There was a problem hiding this comment.
I see. It is CreateLivenessSessionContent now renamed to CreateLivenessSessionParameters (and 2 added properties).
And createLivenessWithVerifySession/createLivenessWithVerifySessionWithVerifyImage API now need a different model, hence CreateLivenessWithVerifySessionParameters (and further 2 more properties).
The versioning of these models is hard, when rename and add is mixed together (e.g. CreateLivenessWithVerifySessionParameters is added in 1.2, but it cannot be annotated as such, since it now being used in API exists in 1.1). I don't have a good recommendation either. As both version is preview (and no REST API change in 1.1), maybe we can just make the break.
Maybe just announce it in SDK thread and see if any dev having issue with this.
weidongxu-microsoft
approved these changes
Jul 24, 2024
Han-msft
added
the
PublishToCustomers
weidongxu-microsoft
added
the
APIStewardshipBoard-SignedOff
|
Approved for JS model's breakings since it introduced better naming. Another hand, JS RLC would recommend inline usage without direct referencing the model type so actually this won't not be a breaking for them. |
3 tasks
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.
Data Plane API Specification Update Pull Request
Tip
Overwhelmed by all this guidance? See the
Getting helpsection at the bottom of this PR description.PR review workflow diagram
Please understand this diagram before proceeding. It explains how to get your PR approved & merged.
API Info: The Basics
Most of the information about your service should be captured in the issue that serves as your API Spec engagement record.
Is this review for (select one):
Change Scope
This section will help us focus on the specific parts of your API that are new or have been modified.
Please share a link to the design document for the new APIs, a link to the previous API Spec document (if applicable), and the root paths that have been updated.
Viewing API changes
For convenient view of the API changes made by this PR, refer to the URLs provided in the table
in the
Generated ApiViewcomment added to this PR. You can use ApiView to show API versions diff.Suppressing failures
If one or multiple validation error/warning suppression(s) is detected in your PR, please follow the
Swagger-Suppression-Process
to get approval.
❔Got questions? Need additional info?? We are here to help!
Contact us!
The Azure API Review Board is dedicated to helping you create amazing APIs. You can read about our mission and learn more about our process on our wiki.
Click here for links to tools, specs, guidelines & other good stuff
Tooling
Guidelines & Specifications
Helpful Links
Getting help
write accessper aka.ms/azsdk/access#request-access-to-rest-api-or-sdk-repositoriesNext Steps to Mergecomment. It will appear within few minutes of submitting this PR and will continue to be up-to-date with current PR state.and https://aka.ms/ci-fix.
queuedstate, please add a comment with contents/azp run.This should result in a new comment denoting a
PR validation pipelinehas started and the checks should be updated after few minutes.