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

Migrate existing Azure AI Face service to TypeSpec #27576

Merged
merged 46 commits into from
Apr 30, 2024
Merged

Migrate existing Azure AI Face service to TypeSpec #27576

merged 46 commits into from
Apr 30, 2024

Conversation

Han-msft
Copy link
Member

@Han-msft Han-msft commented Jan 30, 2024 •
edited
Loading

Data Plane API - Pull Request

Face API is an existing service & restful API for 5+ years as documented in What is the Azure AI Face service?. We continue to extend with new API and the latest addition was Detect liveness in faces that was released in Ignite in Nov 2023 as a Public Preview. At that time, we wanted to release a REST SDK for detect liveness too. However, we were told we cannot use OpenAPI spec that we used to create SDK for old APIs like Detect/Verify/Identify, etc. anymore and TypeSpec is the only option to release new SDK. Our current objective is to release an All-In-One SDK for all the APIs by using TypeSpec, including the existing Face Detect, Verify, Identify, FindSimilar, and the latest detect liveness.

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.

  • Link to API Spec engagement record issue:

Is this review for (select one):

  • a private preview
  • a public preview
  • GA release

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 ApiView comment 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

Checks stuck in `queued` state? If the PR CI checks appear to be stuck in `queued` state, please add a comment with contents `/azp run`. This should result in a new comment denoting a `PR validation pipeline` has started and the checks should be updated after few minutes.

@openapi-pipeline-app openapi-pipeline-app
Copy link

openapi-pipeline-app bot commented Jan 30, 2024 •
edited
Loading

Next Steps to Merge

✅ All automated merging requirements have been met! To get your PR merged, see aka.ms/azsdk/specreview/merge.

@openapi-pipeline-app openapi-pipeline-app
Copy link

openapi-pipeline-app bot commented Jan 30, 2024 •
edited
Loading

Swagger Validation Report

️️✔️BreakingChange succeeded [Detail] [Expand] 
There are no breaking changes.
️❌Breaking Change(Cross-Version): 329 Errors, 219 Warnings failed [Detail]
Compared specs (v0.10.8) new version base version
Face.json v1.1-preview.1(482ba27) v1.0(main)

The following breaking changes are detected by comparison with the latest stable version:

Only 30 items are listed, please refer to log for more details.

Rule Message
❌ 1005 - RemovedPath 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
❌ 1005 - RemovedPath 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
❌ 1005 - RemovedPath 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
❌ 1005 - RemovedPath 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
❌ 1005 - RemovedPath 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
❌ 1005 - RemovedPath 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
❌ 1005 - RemovedPath 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
❌ 1005 - RemovedPath 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
❌ 1005 - RemovedPath 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
❌ 1006 - RemovedDefinition The new version is missing a definition that was found in the old version. Was 'FindSimilarRequest' removed or renamed?
New: Face/preview/v1.1-preview.1/Face.json#L7255:3
Old: Face/stable/v1.0/Face.json#L2832:3
❌ 1006 - RemovedDefinition The new version is missing a definition that was found in the old version. Was 'GroupRequest' removed or renamed?
New: Face/preview/v1.1-preview.1/Face.json#L7255:3
Old: Face/stable/v1.0/Face.json#L2832:3
❌ 1006 - RemovedDefinition The new version is missing a definition that was found in the old version. Was 'IdentifyRequest' removed or renamed?
New: Face/preview/v1.1-preview.1/Face.json#L7255:3
Old: Face/stable/v1.0/Face.json#L2832:3
❌ 1006 - RemovedDefinition The new version is missing a definition that was found in the old version. Was 'VerifyFaceToPersonRequest' removed or renamed?
New: Face/preview/v1.1-preview.1/Face.json#L7255:3
Old: Face/stable/v1.0/Face.json#L2832:3
❌ 1006 - RemovedDefinition The new version is missing a definition that was found in the old version. Was 'VerifyFaceToFaceRequest' removed or renamed?
New: Face/preview/v1.1-preview.1/Face.json#L7255:3
Old: Face/stable/v1.0/Face.json#L2832:3
❌ 1006 - RemovedDefinition The new version is missing a definition that was found in the old version. Was 'UpdateFaceRequest' removed or renamed?
New: Face/preview/v1.1-preview.1/Face.json#L7255:3
Old: Face/stable/v1.0/Face.json#L2832:3
❌ 1006 - RemovedDefinition The new version is missing a definition that was found in the old version. Was 'Confidence' removed or renamed?
New: Face/preview/v1.1-preview.1/Face.json#L7255:3
Old: Face/stable/v1.0/Face.json#L2832:3
❌ 1006 - RemovedDefinition The new version is missing a definition that was found in the old version. Was 'NameAndUserDataContract' removed or renamed?
New: Face/preview/v1.1-preview.1/Face.json#L7255:3
Old: Face/stable/v1.0/Face.json#L2832:3
❌ 1006 - RemovedDefinition The new version is missing a definition that was found in the old version. Was 'MetaDataContract' removed or renamed?
New: Face/preview/v1.1-preview.1/Face.json#L7255:3
Old: Face/stable/v1.0/Face.json#L2832:3
❌ 1006 - RemovedDefinition The new version is missing a definition that was found in the old version. Was 'ApplyScope' removed or renamed?
New: Face/preview/v1.1-preview.1/Face.json#L7255:3
Old: Face/stable/v1.0/Face.json#L2832:3
❌ 1006 - RemovedDefinition The new version is missing a definition that was found in the old version. Was 'ApplySnapshotRequest' removed or renamed?
New: Face/preview/v1.1-preview.1/Face.json#L7255:3
Old: Face/stable/v1.0/Face.json#L2832:3
❌ 1006 - RemovedDefinition The new version is missing a definition that was found in the old version. Was 'TakeSnapshotRequest' removed or renamed?
New: Face/preview/v1.1-preview.1/Face.json#L7255:3
Old: Face/stable/v1.0/Face.json#L2832:3
❌ 1006 - RemovedDefinition The new version is missing a definition that was found in the old version. Was 'UpdateSnapshotRequest' removed or renamed?
New: Face/preview/v1.1-preview.1/Face.json#L7255:3
Old: Face/stable/v1.0/Face.json#L2832:3
❌ 1006 - RemovedDefinition The new version is missing a definition that was found in the old version. Was 'ImageUrl' removed or renamed?
New: Face/preview/v1.1-preview.1/Face.json#L7255:3
Old: Face/stable/v1.0/Face.json#L2832:3
❌ 1007 - RemovedClientParameter The new version is missing a client parameter that was found in the old version. Was 'returnFaceAttributes' removed or renamed?
New: Face/preview/v1.1-preview.1/Face.json#L9398:3
Old: Face/stable/v1.0/Face.json#L4272:3
❌ 1007 - RemovedClientParameter The new version is missing a client parameter that was found in the old version. Was 'targetFace' removed or renamed?
New: Face/preview/v1.1-preview.1/Face.json#L9398:3
Old: Face/stable/v1.0/Face.json#L4272:3
❌ 1007 - RemovedClientParameter The new version is missing a client parameter that was found in the old version. Was 'faceUserData' removed or renamed?
New: Face/preview/v1.1-preview.1/Face.json#L9398:3
Old: Face/stable/v1.0/Face.json#L4272:3
❌ 1007 - RemovedClientParameter The new version is missing a client parameter that was found in the old version. Was 'faceListId' removed or renamed?
New: Face/preview/v1.1-preview.1/Face.json#L9398:3
Old: Face/stable/v1.0/Face.json#L4272:3
❌ 1007 - RemovedClientParameter The new version is missing a client parameter that was found in the old version. Was 'personGroupId' removed or renamed?
New: Face/preview/v1.1-preview.1/Face.json#L9398:3
Old: Face/stable/v1.0/Face.json#L4272:3
❌ 1007 - RemovedClientParameter The new version is missing a client parameter that was found in the old version. Was 'largeFaceListId' removed or renamed?
New: Face/preview/v1.1-preview.1/Face.json#L9398:3
Old: Face/stable/v1.0/Face.json#L4272:3
❌ 1007 - RemovedClientParameter The new version is missing a client parameter that was found in the old version. Was 'largePersonGroupId' removed or renamed?
New: Face/preview/v1.1-preview.1/Face.json#L9398:3
Old: Face/stable/v1.0/Face.json#L4272:3
️️✔️CredScan succeeded [Detail] [Expand] 
There is no credential detected.
️⚠️LintDiff: 113 Warnings warning [Detail]
Compared specs (v2.2.1) new version base version
v1.1-preview.1 v1.1-preview.1(482ba27) default(main)

[must fix]The following errors/warnings are introduced by current PR:

Only 30 items are listed, please refer to log for more details.

Rule Message Related RPC [For API reviewers]
⚠️ EnumInsteadOfBoolean 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
⚠️ EnumInsteadOfBoolean 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
⚠️ EnumInsteadOfBoolean 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
⚠️ VersionPolicy Operation does not define an 'api-version' query parameter.
Location: Face/preview/v1.1-preview.1/Face.json#L354
⚠️ VersionPolicy Operation does not define an 'api-version' query parameter.
Location: Face/preview/v1.1-preview.1/Face.json#L407
⚠️ VersionPolicy Operation does not define an 'api-version' query parameter.
Location: Face/preview/v1.1-preview.1/Face.json#L448
⚠️ PathParameterSchema Path parameter should specify a maximum length (maxLength) and characters allowed (pattern).
Location: Face/preview/v1.1-preview.1/Face.json#L449
⚠️ VersionPolicy Operation does not define an 'api-version' query parameter.
Location: Face/preview/v1.1-preview.1/Face.json#L487
⚠️ PathParameterSchema Path parameter should specify a maximum length (maxLength) and characters allowed (pattern).
Location: Face/preview/v1.1-preview.1/Face.json#L488
⚠️ Delete204Response A delete operation should have a 204 response.
Location: Face/preview/v1.1-preview.1/Face.json#L496
⚠️ SuccessResponseBody All success responses except 202 & 204 should define a response body.
Location: Face/preview/v1.1-preview.1/Face.json#L497
⚠️ VersionPolicy Operation does not define an 'api-version' query parameter.
Location: Face/preview/v1.1-preview.1/Face.json#L524
⚠️ PathParameterSchema Path parameter should specify a maximum length (maxLength) and characters allowed (pattern).
Location: Face/preview/v1.1-preview.1/Face.json#L525
⚠️ VersionPolicy Operation does not define an 'api-version' query parameter.
Location: Face/preview/v1.1-preview.1/Face.json#L586
⚠️ VersionPolicy Operation does not define an 'api-version' query parameter.
Location: Face/preview/v1.1-preview.1/Face.json#L642
⚠️ Formdata Check for appropriate use of formData parameters.
Location: Face/preview/v1.1-preview.1/Face.json#L643
⚠️ Formdata Check for appropriate use of formData parameters.
Location: Face/preview/v1.1-preview.1/Face.json#L646
⚠️ VersionPolicy Operation does not define an 'api-version' query parameter.
Location: Face/preview/v1.1-preview.1/Face.json#L681
⚠️ PathParameterSchema Path parameter should specify a maximum length (maxLength) and characters allowed (pattern).
Location: Face/preview/v1.1-preview.1/Face.json#L682
⚠️ VersionPolicy Operation does not define an 'api-version' query parameter.
Location: Face/preview/v1.1-preview.1/Face.json#L720
⚠️ PathParameterSchema Path parameter should specify a maximum length (maxLength) and characters allowed (pattern).
Location: Face/preview/v1.1-preview.1/Face.json#L721
⚠️ Delete204Response A delete operation should have a 204 response.
Location: Face/preview/v1.1-preview.1/Face.json#L729
⚠️ SuccessResponseBody All success responses except 202 & 204 should define a response body.
Location: Face/preview/v1.1-preview.1/Face.json#L730
⚠️ VersionPolicy Operation does not define an 'api-version' query parameter.
Location: Face/preview/v1.1-preview.1/Face.json#L757
⚠️ PathParameterSchema Path parameter should specify a maximum length (maxLength) and characters allowed (pattern).
Location: Face/preview/v1.1-preview.1/Face.json#L758
⚠️ VersionPolicy Operation does not define an 'api-version' query parameter.
Location: Face/preview/v1.1-preview.1/Face.json#L819
⚠️ VersionPolicy Operation does not define an 'api-version' query parameter.
Location: Face/preview/v1.1-preview.1/Face.json#L875
⚠️ VersionPolicy Operation does not define an 'api-version' query parameter.
Location: Face/preview/v1.1-preview.1/Face.json#L917
⚠️ PatchContentType A patch operation should consume 'application/merge-patch+json' content type.
Location: Face/preview/v1.1-preview.1/Face.json#L993
⚠️ VersionPolicy Operation does not define an 'api-version' query parameter.
Location: Face/preview/v1.1-preview.1/Face.json#L997


The following errors/warnings exist before current PR submission:

Only 30 items are listed, please refer to log for more details.

Rule Message
⚠️ VersionConvention 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
⚠️ VersionPolicy Operation does not define an 'api-version' query parameter.
Location: Face/preview/v1.1-preview.1/Face.json#L92
⚠️ VersionPolicy Operation does not define an 'api-version' query parameter.
Location: Face/preview/v1.1-preview.1/Face.json#L1189
⚠️ VersionPolicy Operation does not define an 'api-version' query parameter.
Location: Face/preview/v1.1-preview.1/Face.json#L1234
⚠️ VersionPolicy Operation does not define an 'api-version' query parameter.
Location: Face/preview/v1.1-preview.1/Face.json#L1284
⚠️ SuccessResponseBody All success responses except 202 & 204 should define a response body.
Location: Face/preview/v1.1-preview.1/Face.json#L1358
⚠️ VersionPolicy Operation does not define an 'api-version' query parameter.
Location: Face/preview/v1.1-preview.1/Face.json#L1383
⚠️ SuccessResponseBody All success responses except 202 & 204 should define a response body.
Location: Face/preview/v1.1-preview.1/Face.json#L1417
⚠️ VersionPolicy Operation does not define an 'api-version' query parameter.
Location: Face/preview/v1.1-preview.1/Face.json#L1442
⚠️ Delete204Response A delete operation should have a 204 response.
Location: Face/preview/v1.1-preview.1/Face.json#L1454
⚠️ SuccessResponseBody All success responses except 202 & 204 should define a response body.
Location: Face/preview/v1.1-preview.1/Face.json#L1455
⚠️ VersionPolicy Operation does not define an 'api-version' query parameter.
Location: Face/preview/v1.1-preview.1/Face.json#L1483
⚠️ VersionPolicy Operation does not define an 'api-version' query parameter.
Location: Face/preview/v1.1-preview.1/Face.json#L1601
⚠️ PathParameterSchema Path parameter should specify a maximum length (maxLength) and characters allowed (pattern).
Location: Face/preview/v1.1-preview.1/Face.json#L1612
⚠️ Delete204Response A delete operation should have a 204 response.
Location: Face/preview/v1.1-preview.1/Face.json#L1621
⚠️ SuccessResponseBody All success responses except 202 & 204 should define a response body.
Location: Face/preview/v1.1-preview.1/Face.json#L1622
⚠️ VersionPolicy Operation does not define an 'api-version' query parameter.
Location: Face/preview/v1.1-preview.1/Face.json#L1650
⚠️ PaginationResponse Operation might be pageable. Consider adding the x-ms-pageable extension.
Location: Face/preview/v1.1-preview.1/Face.json#L1742
⚠️ VersionPolicy Operation does not define an 'api-version' query parameter.
Location: Face/preview/v1.1-preview.1/Face.json#L1746
⚠️ VersionPolicy Operation does not define an 'api-version' query parameter.
Location: Face/preview/v1.1-preview.1/Face.json#L1800
⚠️ VersionPolicy Operation does not define an 'api-version' query parameter.
Location: Face/preview/v1.1-preview.1/Face.json#L1880
⚠️ VersionPolicy Operation does not define an 'api-version' query parameter.
Location: Face/preview/v1.1-preview.1/Face.json#L1943
⚠️ VersionPolicy Operation does not define an 'api-version' query parameter.
Location: Face/preview/v1.1-preview.1/Face.json#L1993
⚠️ SuccessResponseBody All success responses except 202 & 204 should define a response body.
Location: Face/preview/v1.1-preview.1/Face.json#L2067
⚠️ VersionPolicy Operation does not define an 'api-version' query parameter.
Location: Face/preview/v1.1-preview.1/Face.json#L2092
⚠️ SuccessResponseBody All success responses except 202 & 204 should define a response body.
Location: Face/preview/v1.1-preview.1/Face.json#L2126
⚠️ VersionPolicy Operation does not define an 'api-version' query parameter.
Location: Face/preview/v1.1-preview.1/Face.json#L2152
⚠️ Delete204Response A delete operation should have a 204 response.
Location: Face/preview/v1.1-preview.1/Face.json#L2164
⚠️ SuccessResponseBody All success responses except 202 & 204 should define a response body.
Location: Face/preview/v1.1-preview.1/Face.json#L2165
⚠️ VersionPolicy Operation does not define an 'api-version' query parameter.
Location: Face/preview/v1.1-preview.1/Face.json#L2193
️️✔️Avocado succeeded [Detail] [Expand] 
Validation passes for Avocado.
️️✔️SwaggerAPIView succeeded [Detail] [Expand]
️️✔️TypeSpecAPIView succeeded [Detail] [Expand]
️️✔️ModelValidation succeeded [Detail] [Expand] 
Validation passes for ModelValidation.
️️✔️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]
Posted by Swagger Pipeline | How to fix these errors?

@openapi-pipeline-app openapi-pipeline-app
Copy link

openapi-pipeline-app bot commented Jan 30, 2024 •
edited
Loading

Swagger Generation Artifacts

️🔄ApiDocPreview inProgress [Detail]
️⚠️ azure-sdk-for-python warning [Detail]
  • ⚠️Warning in generating from 37acfe2967e5e1be1169146ac461eb1875c9476e. SDK Automation 14.0.0 
    command	sh scripts/automation_init.sh ../azure-sdk-for-python_tmp/initInput.json ../azure-sdk-for-python_tmp/initOutput.json
cmderr	[automation_init.sh] WARNING: Skipping azure-nspkg as it is not installed.
cmderr	[automation_init.sh]
cmderr	[automation_init.sh] npm notice New minor version of npm available! 10.5.0 -> 10.6.0
cmderr	[automation_init.sh] npm notice Changelog: <https://github.com/npm/cli/releases/tag/v10.6.0>
cmderr	[automation_init.sh] npm notice Run `npm install -g npm@10.6.0` to update!
cmderr	[automation_init.sh] npm notice
warn		specification/ai/data-plane/Face/readme.md skipped due to azure-sdk-for-python not found in swagger-to-sdk
command	sh scripts/automation_generate.sh ../azure-sdk-for-python_tmp/generateInput.json ../azure-sdk-for-python_tmp/generateOutput.json
  • ️✔️azure-ai-vision-face [Preview SDK Changes]
    • azure_ai_vision_face-1.0.0b1-py3-none-any.whl
    • azure-ai-vision-face-1.0.0b1.zip
    info	[Changelog] data-plan skip changelog generation temporarily
️❌ azure-sdk-for-net-track2 failed [Detail]
  • ❌Pipeline Framework Failed in generating from 37acfe2967e5e1be1169146ac461eb1875c9476e. SDK Automation 14.0.0 
    command	pwsh ./eng/scripts/Automation-Sdk-Init.ps1 ../azure-sdk-for-net_tmp/initInput.json ../azure-sdk-for-net_tmp/initOutput.json
warn		specification/ai/data-plane/Face/readme.md skipped due to azure-sdk-for-net-track2 not found in swagger-to-sdk
command	pwsh ./eng/scripts/Invoke-GenerateAndBuildV2.ps1 ../azure-sdk-for-net_tmp/generateInput.json ../azure-sdk-for-net_tmp/generateOutput.json
cmderr	[Invoke-GenerateAndBuildV2.ps1]
cmderr	[Invoke-GenerateAndBuildV2.ps1] npm notice New minor version of npm available! 10.5.0 -> 10.6.0
cmderr	[Invoke-GenerateAndBuildV2.ps1] npm notice Changelog: <https://github.com/npm/cli/releases/tag/v10.6.0>
cmderr	[Invoke-GenerateAndBuildV2.ps1] npm notice Run `npm install -g npm@10.6.0` to update!
cmderr	[Invoke-GenerateAndBuildV2.ps1] npm notice
cmderr	[Invoke-GenerateAndBuildV2.ps1] System.InvalidOperationException: The parameter object is marked as Spread but its type is not ModelTypeProvider (got object
cmderr	[Invoke-GenerateAndBuildV2.ps1] )
cmderr	[Invoke-GenerateAndBuildV2.ps1]    at AutoRest.CSharp.Output.Models.OperationMethodChainBuilder.BuildConvenienceMethod(Boolean shouldRequestContextOptional, ConvenienceMethodGenerationInfo generationInfo) in D:\a\_work\1\s\autorest.csharp\src\AutoRest.CSharp\LowLevel\Output\OperationMethodChainBuilder.cs:line 378
cmderr	[Invoke-GenerateAndBuildV2.ps1]    at AutoRest.CSharp.Output.Models.OperationMethodChainBuilder.BuildOperationMethodChain() in D:\a\_work\1\s\autorest.csharp\src\AutoRest.CSharp\LowLevel\Output\OperationMethodChainBuilder.cs:line 106
cmderr	[Invoke-GenerateAndBuildV2.ps1]    at AutoRest.CSharp.Output.Models.LowLevelClient.BuildMethods(LowLevelClient client, TypeFactory typeFactory, IEnumerable`1 operations, ClientFields fields, String namespaceName, String clientName, SourceInputModel sourceInputModel)+MoveNext() in D:\a\_work\1\s\autorest.csharp\src\AutoRest.CSharp\LowLevel\Output\LowLevelClient.cs:line 131
cmderr	[Invoke-GenerateAndBuildV2.ps1]    at System.Collections.Generic.LargeArrayBuilder`1.AddRange(IEnumerable`1 items)
cmderr	[Invoke-GenerateAndBuildV2.ps1]    at System.Collections.Generic.EnumerableHelpers.ToArray[T](IEnumerable`1 source)
cmderr	[Invoke-GenerateAndBuildV2.ps1]    at AutoRest.CSharp.Output.Models.LowLevelClient.get_AllClientMethods() in D:\a\_work\1\s\autorest.csharp\src\AutoRest.CSharp\LowLevel\Output\LowLevelClient.cs:line 95
cmderr	[Invoke-GenerateAndBuildV2.ps1]    at AutoRest.CSharp.Output.Models.LowLevelClient.get_ClientMethods() in D:\a\_work\1\s\autorest.csharp\src\AutoRest.CSharp\LowLevel\Output\LowLevelClient.cs:line 98
cmderr	[Invoke-GenerateAndBuildV2.ps1]    at AutoRest.CSharp.Generation.Writers.DpgClientWriter.WriteClient() in D:\a\_work\1\s\autorest.csharp\src\AutoRest.CSharp\LowLevel\Generation\DpgClientWriter.cs:line 63
cmderr	[Invoke-GenerateAndBuildV2.ps1]    at AutoRest.CSharp.AutoRest.Plugins.LowLevelTarget.ExecuteAsync(GeneratedCodeWorkspace project, InputNamespace inputNamespace, SourceInputModel sourceInputModel, Boolean isTspInput) in D:\a\_work\1\s\autorest.csharp\src\AutoRest.CSharp\LowLevel\AutoRest\LowLevelTarget.cs:line 40
cmderr	[Invoke-GenerateAndBuildV2.ps1]    at AutoRest.CSharp.AutoRest.Plugins.CSharpGen.ExecuteAsync(InputNamespace rootNamespace) in D:\a\_work\1\s\autorest.csharp\src\AutoRest.CSharp\Common\AutoRest\Plugins\CSharpGen.cs:line 91
cmderr	[Invoke-GenerateAndBuildV2.ps1]    at AutoRest.CSharp.AutoRest.Communication.StandaloneGeneratorRunner.RunAsync(CommandLineOptions options) in D:\a\_work\1\s\autorest.csharp\src\AutoRest.CSharp\Common\AutoRest\Communication\StandaloneGeneratorRunner.cs:line 51
cmderr	[Invoke-GenerateAndBuildV2.ps1]    at AutoRest.CSharp.Program.Run(CommandLineOptions options) in D:\a\_work\1\s\autorest.csharp\src\AutoRest.CSharp\Program.cs:line 47
cmderr	[Invoke-GenerateAndBuildV2.ps1]    at AutoRest.CSharp.Program.<>c.<<Main>b__2_0>d.MoveNext() in D:\a\_work\1\s\autorest.csharp\src\AutoRest.CSharp\Program.cs:line 29
cmderr	[Invoke-GenerateAndBuildV2.ps1] --- End of stack trace from previous location ---
cmderr	[Invoke-GenerateAndBuildV2.ps1]    at AutoRest.CSharp.Program.Main(String[] args) in D:\a\_work\1\s\autorest.csharp\src\AutoRest.CSharp\Program.cs:line 26
cmderr	[Invoke-GenerateAndBuildV2.ps1]    at AutoRest.CSharp.Program.<Main>(String[] args)
cmderr	[Invoke-GenerateAndBuildV2.ps1] Aborted (core dumped)
cmderr	[Invoke-GenerateAndBuildV2.ps1] Emitter "@azure-tools/typespec-csharp" crashed! This is a bug.
cmderr	[Invoke-GenerateAndBuildV2.ps1] Please file an issue at https://github.com/Azure/autorest.csharp/issues
cmderr	[Invoke-GenerateAndBuildV2.ps1] Error: Command failed: dotnet --roll-forward Major /mnt/vss/_work/1/s/azure-sdk-for-net/sdk/face/Azure.AI.Vision.Face/TempTypeSpecFiles/Face/node_modules/@autorest/csharp/AutoRest.CSharp.dll --project-path /mnt/vss/_work/1/s/azure-sdk-for-net/sdk/face/Azure.AI.Vision.Face/src --new-project  --clear-output-folder true
cmderr	[Invoke-GenerateAndBuildV2.ps1]     at checkExecSyncError (node:child_process:890:11)
cmderr	[Invoke-GenerateAndBuildV2.ps1]     at execSync (node:child_process:962:15)
cmderr	[Invoke-GenerateAndBuildV2.ps1]     at Object.$onEmit [as emitFunction] (file:///mnt/vss/_work/1/s/azure-sdk-for-net/sdk/face/Azure.AI.Vision.Face/TempTypeSpecFiles/Face/node_modules/@azure-tools/typespec-csharp/dist/src/emitter.js:128:21)
cmderr	[Invoke-GenerateAndBuildV2.ps1]     at async runEmitter (file:///mnt/vss/_work/1/s/azure-sdk-for-net/sdk/face/Azure.AI.Vision.Face/TempTypeSpecFiles/Face/node_modules/@typespec/compiler/dist/src/core/program.js:555:13)
cmderr	[Invoke-GenerateAndBuildV2.ps1]     at async compile (file:///mnt/vss/_work/1/s/azure-sdk-for-net/sdk/face/Azure.AI.Vision.Face/TempTypeSpecFiles/Face/node_modules/@typespec/compiler/dist/src/core/program.js:238:9)
cmderr	[Invoke-GenerateAndBuildV2.ps1]     at async compileOnce (file:///mnt/vss/_work/1/s/azure-sdk-for-net/sdk/face/Azure.AI.Vision.Face/TempTypeSpecFiles/Face/node_modules/@typespec/compiler/dist/src/core/cli/actions/compile/compile.js:37:25)
cmderr	[Invoke-GenerateAndBuildV2.ps1]     at async compileAction (file:///mnt/vss/_work/1/s/azure-sdk-for-net/sdk/face/Azure.AI.Vision.Face/TempTypeSpecFiles/Face/node_modules/@typespec/compiler/dist/src/core/cli/actions/compile/compile.js:21:9)
cmderr	[Invoke-GenerateAndBuildV2.ps1] --------------------------------------------------
cmderr	[Invoke-GenerateAndBuildV2.ps1] Library Version                0.2.0-beta.20240428.10
cmderr	[Invoke-GenerateAndBuildV2.ps1] TypeSpec Compiler Version      0.55.0
cmderr	[Invoke-GenerateAndBuildV2.ps1] --------------------------------------------------
cmderr	[Invoke-GenerateAndBuildV2.ps1] Write-Error: /mnt/vss/_work/1/s/azure-sdk-for-net/eng/scripts/Invoke-GenerateAndBuildV2.ps1:120
cmderr	[Invoke-GenerateAndBuildV2.ps1] Line |
cmderr	[Invoke-GenerateAndBuildV2.ps1]  120 |  … ectFolder = & $processScript $typespecFolder $commitid $repoHttpsUrl  …
cmderr	[Invoke-GenerateAndBuildV2.ps1]      |                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cmderr	[Invoke-GenerateAndBuildV2.ps1]      | Failed to generate sdk project at
cmderr	[Invoke-GenerateAndBuildV2.ps1]      | /mnt/vss/_work/1/s/azure-sdk-for-net/sdk/face/Azure.AI.Vision.Face
fatal: 'sdkAuto/27576/' is not a valid branch name
  • ❌
    error	Fatal error: fatal: 'sdkAuto/27576/' is not a valid branch name
error	Fatal error: Error: fatal: 'sdkAuto/27576/' is not a valid branch name    at Object.action (/mnt/vss/_work/1/a/unified-pipeline-runtime/common/temp/node_modules/.pnpm/simple-git@3.22.0/node_modules/simple-git/dist/cjs/index.js:1332:25)    at PluginStore.exec (/mnt/vss/_work/1/a/unified-pipeline-runtime/common/temp/node_modules/.pnpm/simple-git@3.22.0/node_modules/simple-git/dist/cjs/index.js:1367:29)    at /mnt/vss/_work/1/a/unified-pipeline-runtime/common/temp/node_modules/.pnpm/simple-git@3.22.0/node_modules/simple-git/dist/cjs/index.js:1766:43    at new Promise (<anonymous>)    at GitExecutorChain.handleTaskData (/mnt/vss/_work/1/a/unified-pipeline-runtime/common/temp/node_modules/.pnpm/simple-git@3.22.0/node_modules/simple-git/dist/cjs/index.js:1764:16)    at GitExecutorChain.<anonymous> (/mnt/vss/_work/1/a/unified-pipeline-runtime/common/temp/node_modules/.pnpm/simple-git@3.22.0/node_modules/simple-git/dist/cjs/index.js:1748:44)    at Generator.next (<anonymous>)    at fulfilled (/mnt/vss/_work/1/a/unified-pipeline-runtime/common/temp/node_modules/.pnpm/simple-git@3.22.0/node_modules/simple-git/dist/cjs/index.js:55:24)
error	The following packages are still pending:
error
️🔄 azure-sdk-for-js inProgress [Detail]
️⚠️ azure-sdk-for-java warning [Detail]
  • ⚠️Warning in generating from 37acfe2967e5e1be1169146ac461eb1875c9476e. SDK Automation 14.0.0 
    command	./eng/mgmt/automation/init.sh ../azure-sdk-for-java_tmp/initInput.json ../azure-sdk-for-java_tmp/initOutput.json
cmderr	[init.sh]  notice
cmderr	[init.sh] npm notice New minor version of npm available! 10.5.0 -> 10.6.0
cmderr	[init.sh] npm notice Changelog: <https://github.com/npm/cli/releases/tag/v10.6.0>
cmderr	[init.sh] npm notice Run `npm install -g npm@10.6.0` to update!
cmderr	[init.sh] npm notice
warn		specification/ai/data-plane/Face/readme.md skipped due to azure-sdk-for-java not found in swagger-to-sdk
command	./eng/mgmt/automation/generate.py ../azure-sdk-for-java_tmp/generateInput.json ../azure-sdk-for-java_tmp/generateOutput.json
  • ️✔️azure-ai-vision-face [Preview SDK Changes]
    • pom.xml
    • azure-ai-vision-face-1.0.0-beta.1-sources.jar
    • azure-ai-vision-face-1.0.0-beta.1.jar
Posted by Swagger Pipeline | How to fix these errors?

@openapi-pipeline-app openapi-pipeline-app
Copy link

openapi-pipeline-app bot commented Jan 30, 2024 •
edited
Loading

Generated ApiView

Language Package Name ApiView Link
Java azure-ai-vision-face https://apiview.dev/Assemblies/Review/572978f1cf2945df8bcf83aa83876685?revisionId=91cf1e7557ff48619947a2ecbb772436
Swagger Face https://apiview.dev/Assemblies/Review/2ab42e93e3c64f34aa8dfb69ea93cf15?revisionId=22e1728ba90145afb78aaca3b0c0f296
TypeSpec Face https://apiview.dev/Assemblies/Review/fd70378562b9470abec3d3f5e04aa38f?revisionId=08b840415ac84325b0e72f4e62abf528

@openapi-workflow-bot openapi-workflow-bot
Copy link

Hi @Han-msft! For review efficiency consideration, when creating a new API version, it is required to place API specs of the base version in the first commit, and push new version updates into successive commits. You can use OpenAPIHub to initialize the PR for adding a new version.
For more details refer to the wiki.

This was referenced Jan 30, 2024
Closed
Closed
Closed
Closed
@msyyc msyyc mentioned this pull request Feb 5, 2024
Closed
mikekistler
mikekistler requested changes Feb 21, 2024
View reviewed changes
Copy link
Member

@mikekistler mikekistler left a comment

Choose a reason for hiding this comment

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

Found a few issues I hope you can address.

specification/ai/data-plane/Face/preview/v1.1-preview.1/Face.json
Comment on lines +214 to +358
"schema": {
"type": "array",
"items": {
"$ref": "#/definitions/LivenessSessionItem"
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Response schema should not be a bare array.

Copy link
Member Author

Choose a reason for hiding this comment

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

This API is already public previewed. We prefer not to change it.

specification/ai/data-plane/Face/preview/v1.1-preview.1/Face.json Outdated
"/face/{apiVersion}/detectLiveness/singleModal/sessions": {
"get": {
"operationId": "LivenessSessions_ListSessions",
"description": "Operation that lists resources in a paginated way.",
Copy link
Member

Choose a reason for hiding this comment

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

How does pagination work for this API?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's the description that comes from the TypeSpec Azure.Core operation. Our list operation only support query by top and start, I think it's not fulfilling the pagination of Azure API. I'll override the description.

specification/ai/data-plane/Face/preview/v1.1-preview.1/Face.json Outdated
"ListRequestOptions.top": {
"name": "top",
"in": "query",
"description": "The number of sessions to list, ranging in [1, 1000]. Default is 1000.",
Copy link
Member

Choose a reason for hiding this comment

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

Top should not have a default value. The default is to return all elements of the collection, paginated as needed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Our long time ago GAed API use these parameters for list request. We'll consider revise it in our next generation service but not changing it for now.

specification/ai/data-plane/Face/preview/v1.1-preview.1/Face.json Outdated Show resolved Hide resolved
specification/ai/data-plane/Face/preview/v1.1-preview.1/Face.json Outdated
},
"tags": [],
"paths": {
"/face/{apiVersion}/compare": {
Copy link
Member

Choose a reason for hiding this comment

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

I think you should move "/face/{apiVersion}" into the x-ms-parameterized-host, as it was in your GA API. That will be more DRY and also make comparison across API versions easier.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated. However, Azure.Core.Foundations.ApiVersionParameter is injected to all our API's parameter, I could not find a way to remove it.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm going to rollback the change since there is no way to remove ApiVersionParameter in query if we don't override it by putting it in request path.

specification/ai/data-plane/Face/preview/v1.1-preview.1/Face.json Outdated Show resolved Hide resolved
specification/ai/data-plane/Face/preview/v1.1-preview.1/Face.json
@@ -0,0 +1,7948 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

I think you should populate the GA API in the "stable/v1.0" folder, to establish the baseline for tooling.

There are two ways to do this. One is to make your TypeSpec versioned, with a v1.0 version and a v1.1-preview version. I think this is the preferred solution. Or you could just copy the directory over from the cognitiveservices directory as is.

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought we only need to support latest version when migrating to TypeSpec. That's what I observed from document intelligence and image analysis. I'll go for copy the swagger file. Please let me know if you think we should include them in TypeSpec.

Copy link
Member Author

Choose a reason for hiding this comment

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

I copied the stable swagger to current ai folder. How do I know whether it works for tooling? Would it take effect on APIView or it's other tool?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm going to revert the swagger commit for now. Because Jeffery said he is going to check more detail about folder change of cognitive service. Copying swagger also bring more validation failure.

heaths
heaths approved these changes Feb 21, 2024
View reviewed changes
Copy link
Member

@heaths heaths left a comment

Choose a reason for hiding this comment

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

Approved based on the changes we discussed in the meeting, but for a v2 there's a lot to consider here. More than I've reviewed since this PR was not the one listed in the meeting request for review (and I reviewed that one, which was significantly smaller).

specification/ai/Face/models.session.tsp Outdated
}

@doc("Session status.")
enum SessionStatus {
Copy link
Member

Choose a reason for hiding this comment

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

If this is part of a long-running operation, we have standard enums we recommend: https://github.com/microsoft/api-guidelines/blob/vNext/azure/Guidelines.md#lro-status-monitor-structure. Even if it's not for an LRO, I would use the same value for familiarity.

Copy link
Member Author

Choose a reason for hiding this comment

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

No, it's not the status of operation. It stands for the lifecycle of created session object.

specification/ai/Face/models.session.tsp Outdated
Comment on lines 182 to 278
CreationContent: LivenessSessionCreationContentForMultipart;

#suppress "@azure-tools/typespec-azure-core/casing-style"
@doc("The image stream for verify.")
VerifyImage: bytes;
Copy link
Member

Choose a reason for hiding this comment

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

Why did you switch to PascalCase for these two properties?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's reflecting the underlying key inside the form-data request of our existing service. Do you prefer to name these properties with camelCase and use decorator to specify it's PascalCase in actual request?

specification/ai/Face/models.common.tsp Outdated
Comment on lines 13 to 32
model ListRequestOptions {
@doc("List resources greater than the \"start\". It contains no more than 64 characters. Default is empty.")
@query
start?: string;

@doc("The number of items to list, ranging in [1, 1000]. Default is 1000.")
@query
top?: int32;
}
Copy link
Member

Choose a reason for hiding this comment

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

These should be query parameters named start skip and top. That's the recommendation, anyway.

Copy link
Member Author

Choose a reason for hiding this comment

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

This would be breaking change to existing service. We should consider in future release.

specification/ai/Face/models.common.tsp Outdated
}

@doc("Type of training status.")
enum TrainingStatusType {
Copy link
Member

Choose a reason for hiding this comment

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

We already have a standard enum for LRO status. See https://azure.github.io/typespec-azure/docs/getstarted/azure-core/step06 for an example. The values you define here aren't correct, for example, and should be PascalCase.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is also for operation GAed a long time ago.

specification/ai/Face/models.common.tsp Outdated Show resolved Hide resolved
specification/ai/Face/routes.common.tsp

namespace Face;

alias ServiceTraits = NoClientRequestId &
Copy link
Member

Choose a reason for hiding this comment

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

Why do you say you don't support a client request ID here but then have a property for it in your audit models? We do recommend supporting client request IDs to assist callers with tracing, etc.

Copy link
Member Author

Choose a reason for hiding this comment

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

We support it but not with header key "x-ms-client-request-id", which is the behavior of SupportsClientRequestId trait.

@Han-msft Han-msft changed the title Azure AI Face TypeSpec Migrate existing Azure AI Face service to TypeSpec Feb 22, 2024
@openapi-pipeline-app openapi-pipeline-app
Copy link

PR validation pipeline can not start as the pull request is not merged or mergeable - most likely it has merge conflicts.

1 similar comment
@openapi-pipeline-app openapi-pipeline-app
Copy link

PR validation pipeline can not start as the pull request is not merged or mergeable - most likely it has merge conflicts.

This was referenced Mar 4, 2024
Closed
Closed
Closed
@openapi-pipeline-app openapi-pipeline-app
Copy link

PR validation pipeline can not start as the pull request is not merged or mergeable - most likely it has merge conflicts.

@Han-msft Han-msft marked this pull request as ready for review March 6, 2024 01:02
@Han-msft Han-msft requested a review from a team as a code owner March 6, 2024 01:02
@Han-msft Han-msft requested review from tg-msft and removed request for a team March 6, 2024 01:02
@zihyunting zihyunting mentioned this pull request Apr 26, 2024
Merged
6 tasks
@v-jiaodi v-jiaodi mentioned this pull request Apr 28, 2024
Merged
@MaryGao
Copy link
Member

MaryGao commented Apr 29, 2024

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 4 pipeline(s).

MaryGao
MaryGao reviewed Apr 30, 2024
View reviewed changes
specification/ai/Face/routes.session.tsp Outdated
@returnsDoc(SessionCreationSuccess)
createLivenessSession is FaceResourceCreateWithServiceProvidedName<
LivenessSession,
CreateLivenessSessionParameters,
Copy link
Member

@MaryGao MaryGao Apr 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Not sure we could rename this model name CreateLivenessSessionParameters.

This is conflicted with RLC's naming rule {Operations}Parameters. We would resolve this conflict in emitter side. To non-block you it would be great if you could rename it.

JS issue tracked: Azure/autorest.typescript#2487

@weidongxu-microsoft weidongxu-microsoft merged commit 241c4f0 into Azure:main Apr 30, 2024
29 of 32 checks passed
This was referenced May 3, 2024
Closed
Merged
@mikekistler mikekistler mentioned this pull request Jul 23, 2024
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MaryGao MaryGao MaryGao left review comments

@mikekistler mikekistler mikekistler approved these changes

@weidongxu-microsoft weidongxu-microsoft weidongxu-microsoft approved these changes

@heaths heaths heaths approved these changes

@tg-msft tg-msft Awaiting requested review from tg-msft tg-msft is a code owner automatically assigned from Azure/api-stewardship-board

Assignees
No one assigned
Labels
APIStewardshipBoard-SignedOff The Azure API Stewardship team has reviewed and approved the changes. Approved-Suppression BreakingChange-Approved-Benign Changes are not breaking at the REST API level and have at most minor impact to generated SDKs. BreakingChangeReviewRequired <valid label in PR review process>add this label when breaking change review is required CI-MissingBaseCommit data-plane new-api-version SuppressionReviewRequired TypeSpec Authored with TypeSpec
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@Han-msft @MaryGao @heaths @weidongxu-microsoft @mikekistler @AzureRestAPISpecReview @shaoli-msft