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.

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

[Hub Generated] Review request for WebPubSub to add version stable/2023-07-01 #23927

Merged
merged 9 commits into from
Jun 16, 2023

Conversation

xingsy97
Copy link
Contributor

@xingsy97 xingsy97 commented May 11, 2023

This is a PR generated at OpenAPI Hub. You can view your work branch via this link.

Data Plane API - Pull Request

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.

  • Design Document:

    • Add an API to add connections to multiple groups. Target connections are specified by a filter string
    • Add an API to remove connections from multiple groups. Target connections are specified by a filter string
    • Add a messageTtlSeconds to Send* APIs.
      • This value defines the expiration time for a message. Messages that are not consumed by the client within the specified TTL will be dropped by the service. This parameter helps when the client's bandwidth is limited.
  • Previous API Spec Doc:

  • Updated paths:

    • Added one /api/hubs/{hub}/:addToGroup
    • Added one /api/hubs/{hub}/:removeFromGroup
    • Added one query messageTtlSeconds for following send* API
      • /api/hubs/{hub}/:send
      • /api/hubs/{hub}/connections/{connectionId}/:send
      • /api/hubs/{hub}/groups/{group}/:send
      • /api/hubs/{hub}/users/{userId}/:send

❔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

fix #24145

@openapi-workflow-bot
Copy link

Hi, @xingsy97 Thanks for your PR. I am workflow bot for review process. Here are some small tips.

  • Please ensure to do self-check against checklists in first PR comment.
  • PR assignee is the person auto-assigned and responsible for your current PR reviewing and merging.
  • For specs comparison cross API versions, Use API Specs Comparison Report Generator
  • If there is CI failure(s), to fix CI error(s) is mandatory for PR merging; or you need to provide justification in PR comment for explanation. How to fix?

  • Any feedback about review process or workflow bot, pls contact swagger and tools team. vscswagger@microsoft.com

    @openapi-pipeline-app
    Copy link

    openapi-pipeline-app bot commented May 11, 2023

    Swagger Validation Report

    ️️✔️BreakingChange succeeded [Detail] [Expand]
    There are no breaking changes.
    ️⚠️Breaking Change(Cross-Version): 49 Warnings warning [Detail]
    compared swaggers (via Oad v0.10.4)] new version base version
    webpubsub.json 2023-07-01(efe2ec0) 2022-11-01(main)
    webpubsub.json 2023-07-01(efe2ec0) 2021-08-01-preview(main)

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

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

    Rule Message
    ⚠️ 1007 - RemovedClientParameter The new version is missing a client parameter that was found in the old version. Was 'Endpoint' removed or renamed?
    Old: WebPubSub/preview/2021-08-01-preview/webpubsub.json#L998:3
    ⚠️ 1011 - AddingResponseCode The new version adds a response code '204'.
    New: WebPubSub/stable/2023-07-01/webpubsub.json#L461:11
    ⚠️ 1011 - AddingResponseCode The new version adds a response code '204'.
    New: WebPubSub/stable/2023-07-01/webpubsub.json#L969:11
    ⚠️ 1011 - AddingResponseCode The new version adds a response code '204'.
    New: WebPubSub/stable/2023-07-01/webpubsub.json#L1626:11
    ⚠️ 1011 - AddingResponseCode The new version adds a response code '204'.
    New: WebPubSub/stable/2023-07-01/webpubsub.json#L1556:11
    ⚠️ 1011 - AddingResponseCode The new version adds a response code '204'.
    New: WebPubSub/stable/2023-07-01/webpubsub.json#L1118:11
    ⚠️ 1025 - RequiredStatusChange The 'required' status changed from the old version('False') to the new version('True').
    New: WebPubSub/stable/2023-07-01/webpubsub.json#L16:11
    Old: WebPubSub/preview/2021-08-01-preview/webpubsub.json#L25:11
    ⚠️ 1025 - RequiredStatusChange The 'required' status changed from the old version('False') to the new version('True').
    New: WebPubSub/stable/2023-07-01/webpubsub.json#L217:11
    Old: WebPubSub/preview/2021-08-01-preview/webpubsub.json#L90:11
    ⚠️ 1025 - RequiredStatusChange The 'required' status changed from the old version('False') to the new version('True').
    New: WebPubSub/stable/2023-07-01/webpubsub.json#L361:11
    Old: WebPubSub/preview/2021-08-01-preview/webpubsub.json#L143:11
    ⚠️ 1025 - RequiredStatusChange The 'required' status changed from the old version('False') to the new version('True').
    New: WebPubSub/stable/2023-07-01/webpubsub.json#L452:11
    Old: WebPubSub/preview/2021-08-01-preview/webpubsub.json#L246:11
    ⚠️ 1025 - RequiredStatusChange The 'required' status changed from the old version('False') to the new version('True').
    New: WebPubSub/stable/2023-07-01/webpubsub.json#L507:11
    Old: WebPubSub/preview/2021-08-01-preview/webpubsub.json#L196:11
    ⚠️ 1025 - RequiredStatusChange The 'required' status changed from the old version('False') to the new version('True').
    New: WebPubSub/stable/2023-07-01/webpubsub.json#L572:11
    Old: WebPubSub/preview/2021-08-01-preview/webpubsub.json#L294:11
    ⚠️ 1025 - RequiredStatusChange The 'required' status changed from the old version('False') to the new version('True').
    New: WebPubSub/stable/2023-07-01/webpubsub.json#L710:11
    Old: WebPubSub/preview/2021-08-01-preview/webpubsub.json#L348:11
    ⚠️ 1025 - RequiredStatusChange The 'required' status changed from the old version('False') to the new version('True').
    New: WebPubSub/stable/2023-07-01/webpubsub.json#L865:11
    Old: WebPubSub/preview/2021-08-01-preview/webpubsub.json#L410:11
    ⚠️ 1025 - RequiredStatusChange The 'required' status changed from the old version('False') to the new version('True').
    New: WebPubSub/stable/2023-07-01/webpubsub.json#L960:11
    Old: WebPubSub/preview/2021-08-01-preview/webpubsub.json#L525:11
    ⚠️ 1025 - RequiredStatusChange The 'required' status changed from the old version('False') to the new version('True').
    New: WebPubSub/stable/2023-07-01/webpubsub.json#L1028:11
    Old: WebPubSub/preview/2021-08-01-preview/webpubsub.json#L472:11
    ⚠️ 1025 - RequiredStatusChange The 'required' status changed from the old version('False') to the new version('True').
    New: WebPubSub/stable/2023-07-01/webpubsub.json#L1321:11
    Old: WebPubSub/preview/2021-08-01-preview/webpubsub.json#L568:11
    ⚠️ 1025 - RequiredStatusChange The 'required' status changed from the old version('False') to the new version('True').
    New: WebPubSub/stable/2023-07-01/webpubsub.json#L1462:11
    Old: WebPubSub/preview/2021-08-01-preview/webpubsub.json#L619:11
    ⚠️ 1025 - RequiredStatusChange The 'required' status changed from the old version('False') to the new version('True').
    New: WebPubSub/stable/2023-07-01/webpubsub.json#L1617:11
    Old: WebPubSub/preview/2021-08-01-preview/webpubsub.json#L734:11
    ⚠️ 1025 - RequiredStatusChange The 'required' status changed from the old version('False') to the new version('True').
    New: WebPubSub/stable/2023-07-01/webpubsub.json#L1685:11
    Old: WebPubSub/preview/2021-08-01-preview/webpubsub.json#L681:11
    ⚠️ 1025 - RequiredStatusChange The 'required' status changed from the old version('False') to the new version('True').
    New: WebPubSub/stable/2023-07-01/webpubsub.json#L1547:11
    Old: WebPubSub/preview/2021-08-01-preview/webpubsub.json#L777:11
    ⚠️ 1025 - RequiredStatusChange The 'required' status changed from the old version('False') to the new version('True').
    New: WebPubSub/stable/2023-07-01/webpubsub.json#L1109:11
    Old: WebPubSub/preview/2021-08-01-preview/webpubsub.json#L903:11
    ⚠️ 1025 - RequiredStatusChange The 'required' status changed from the old version('False') to the new version('True').
    New: WebPubSub/stable/2023-07-01/webpubsub.json#L1185:11
    Old: WebPubSub/preview/2021-08-01-preview/webpubsub.json#L965:11
    ⚠️ 1025 - RequiredStatusChange The 'required' status changed from the old version('False') to the new version('True').
    New: WebPubSub/stable/2023-07-01/webpubsub.json#L1264:11
    Old: WebPubSub/preview/2021-08-01-preview/webpubsub.json#L841:11
    ⚠️ 1027 - DefaultValueChanged The new version has a different default value than the previous one.
    New: WebPubSub/stable/2023-07-01/webpubsub.json#L16:11
    Old: WebPubSub/preview/2021-08-01-preview/webpubsub.json#L25:11
    ⚠️ 1027 - DefaultValueChanged The new version has a different default value than the previous one.
    New: WebPubSub/stable/2023-07-01/webpubsub.json#L192:11
    Old: WebPubSub/preview/2021-08-01-preview/webpubsub.json#L65:11
    ⚠️ 1027 - DefaultValueChanged The new version has a different default value than the previous one.
    New: WebPubSub/stable/2023-07-01/webpubsub.json#L217:11
    Old: WebPubSub/preview/2021-08-01-preview/webpubsub.json#L90:11
    ⚠️ 1027 - DefaultValueChanged The new version has a different default value than the previous one.
    New: WebPubSub/stable/2023-07-01/webpubsub.json#L361:11
    Old: WebPubSub/preview/2021-08-01-preview/webpubsub.json#L143:11
    ⚠️ 1027 - DefaultValueChanged The new version has a different default value than the previous one.
    New: WebPubSub/stable/2023-07-01/webpubsub.json#L452:11
    Old: WebPubSub/preview/2021-08-01-preview/webpubsub.json#L246:11
    ⚠️ 1027 - DefaultValueChanged The new version has a different default value than the previous one.
    New: WebPubSub/stable/2023-07-01/webpubsub.json#L507:11
    Old: WebPubSub/preview/2021-08-01-preview/webpubsub.json#L196:11
    ️️✔️CredScan succeeded [Detail] [Expand]
    There is no credential detected.
    ️⚠️LintDiff: 6 Warnings warning [Detail]
    compared tags (via openapi-validator v2.1.3) new version base version
    package-2023-07-01 package-2023-07-01(efe2ec0) default(main)

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

    Rule Message Related RPC [For API reviewers]
    ⚠️ PathParameterSchema Path parameter should specify a maximum length (maxLength).
    Location: WebPubSub/stable/2023-07-01/webpubsub.json#L53
    ⚠️ SuccessResponseBody All success responses except 202 & 204 should define a response body.
    Location: WebPubSub/stable/2023-07-01/webpubsub.json#L79
    ⚠️ ErrorResponse Error response schema should contain an object property named error.
    Location: WebPubSub/stable/2023-07-01/webpubsub.json#L84
    ⚠️ PathParameterSchema Path parameter should specify a maximum length (maxLength).
    Location: WebPubSub/stable/2023-07-01/webpubsub.json#L277
    ⚠️ SuccessResponseBody All success responses except 202 & 204 should define a response body.
    Location: WebPubSub/stable/2023-07-01/webpubsub.json#L303
    ⚠️ ErrorResponse Error response schema should contain an object property named error.
    Location: WebPubSub/stable/2023-07-01/webpubsub.json#L308


    The following errors/warnings exist before current PR submission:

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

    Rule Message
    LroExtension Operations with a 202 response must specify x-ms-long-running-operation: true.
    Location: WebPubSub/stable/2023-07-01/webpubsub.json#L328
    LroExtension Operations with a 202 response must specify x-ms-long-running-operation: true.
    Location: WebPubSub/stable/2023-07-01/webpubsub.json#L541
    LroExtension Operations with a 202 response must specify x-ms-long-running-operation: true.
    Location: WebPubSub/stable/2023-07-01/webpubsub.json#L822
    LroExtension Operations with a 202 response must specify x-ms-long-running-operation: true.
    Location: WebPubSub/stable/2023-07-01/webpubsub.json#L1431
    ⚠️ SuccessResponseBody All success responses except 202 & 204 should define a response body.
    Location: WebPubSub/stable/2023-07-01/webpubsub.json#L25
    ⚠️ ErrorResponse Error response should have a schema.
    Location: WebPubSub/stable/2023-07-01/webpubsub.json#L28
    ⚠️ ErrorResponse Error response should contain a x-ms-error-code header.
    Location: WebPubSub/stable/2023-07-01/webpubsub.json#L28
    ⚠️ PathParameterSchema Path parameter should specify a maximum length (maxLength).
    Location: WebPubSub/stable/2023-07-01/webpubsub.json#L114
    ⚠️ ErrorResponse Error response schema should contain an object property named error.
    Location: WebPubSub/stable/2023-07-01/webpubsub.json#L152
    ⚠️ PathParameterSchema Path parameter should specify a maximum length (maxLength).
    Location: WebPubSub/stable/2023-07-01/webpubsub.json#L184
    ⚠️ ErrorResponse Error response schema should contain an object property named error.
    Location: WebPubSub/stable/2023-07-01/webpubsub.json#L244
    ⚠️ PathParameterSchema Path parameter should specify a maximum length (maxLength).
    Location: WebPubSub/stable/2023-07-01/webpubsub.json#L343
    ⚠️ LroHeaders A 202 response should include an Operation-Location response header.
    Location: WebPubSub/stable/2023-07-01/webpubsub.json#L395
    ⚠️ ErrorResponse Error response schema should contain an object property named error.
    Location: WebPubSub/stable/2023-07-01/webpubsub.json#L400
    ⚠️ DeleteInOperationName 'DELETE' operation 'WebPubSub_CloseConnection' should use method name 'Delete'. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change.
    Location: WebPubSub/stable/2023-07-01/webpubsub.json#L425
    ⚠️ OperationId OperationId for delete method should contain 'Delete'
    Location: WebPubSub/stable/2023-07-01/webpubsub.json#L425
    ⚠️ PathParameterSchema Path parameter should specify a maximum length (maxLength).
    Location: WebPubSub/stable/2023-07-01/webpubsub.json#L430
    ⚠️ PathParameterSchema Path parameter should specify a maximum length (maxLength) and characters allowed (pattern).
    Location: WebPubSub/stable/2023-07-01/webpubsub.json#L438
    ⚠️ ErrorResponse Error response schema should contain an object property named error.
    Location: WebPubSub/stable/2023-07-01/webpubsub.json#L466
    ⚠️ PathParameterSchema Path parameter should specify a maximum length (maxLength).
    Location: WebPubSub/stable/2023-07-01/webpubsub.json#L491
    ⚠️ PathParameterSchema Path parameter should specify a maximum length (maxLength) and characters allowed (pattern).
    Location: WebPubSub/stable/2023-07-01/webpubsub.json#L499
    ⚠️ SuccessResponseBody All success responses except 202 & 204 should define a response body.
    Location: WebPubSub/stable/2023-07-01/webpubsub.json#L516
    ⚠️ ErrorResponse Error response should have a schema.
    Location: WebPubSub/stable/2023-07-01/webpubsub.json#L519
    ⚠️ ErrorResponse Error response should contain a x-ms-error-code header.
    Location: WebPubSub/stable/2023-07-01/webpubsub.json#L519
    ⚠️ ErrorResponse Error response should contain x-ms-error-response.
    Location: WebPubSub/stable/2023-07-01/webpubsub.json#L519
    ⚠️ ErrorResponse Error response should have a schema.
    Location: WebPubSub/stable/2023-07-01/webpubsub.json#L522
    ⚠️ PathParameterSchema Path parameter should specify a maximum length (maxLength).
    Location: WebPubSub/stable/2023-07-01/webpubsub.json#L556
    ⚠️ PathParameterSchema Path parameter should specify a maximum length (maxLength) and characters allowed (pattern).
    Location: WebPubSub/stable/2023-07-01/webpubsub.json#L564
    ⚠️ LroHeaders A 202 response should include an Operation-Location response header.
    Location: WebPubSub/stable/2023-07-01/webpubsub.json#L600
    ⚠️ ErrorResponse Error response schema should contain an object property named error.
    Location: WebPubSub/stable/2023-07-01/webpubsub.json#L605
    ️️✔️Avocado succeeded [Detail] [Expand]
    Validation passes for Avocado.
    ️️✔️ApiReadinessCheck succeeded [Detail] [Expand]
    ️⚠️~[Staging] ServiceAPIReadinessTest: 0 Warnings warning [Detail]

    API Test is not triggered due to precheck failure. Check pipeline log for details.

    ️️✔️SwaggerAPIView succeeded [Detail] [Expand]
    ️️✔️CadlAPIView 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.
    ️️✔️PrettierCheck succeeded [Detail] [Expand]
    Validation passes for PrettierCheck.
    ️️✔️SpellCheck succeeded [Detail] [Expand]
    Validation passes for SpellCheck.
    ️️✔️Lint(RPaaS) succeeded [Detail] [Expand]
    Validation passes for Lint(RPaaS).
    ️️✔️CadlValidation succeeded [Detail] [Expand]
    Validation passes for CadlValidation.
    ️️✔️TypeSpec Validation succeeded [Detail] [Expand]
    Validation passes for TypeSpec Validation.
    ️️✔️PR Summary succeeded [Detail] [Expand]
    Validation passes for Summary.
    Posted by Swagger Pipeline | How to fix these errors?

    @openapi-pipeline-app
    Copy link

    openapi-pipeline-app bot commented May 11, 2023

    Swagger Generation Artifacts

    ️️✔️ApiDocPreview succeeded [Detail] [Expand]
     Please click here to preview with your @microsoft account. 
    ️️✔️SDK Breaking Change Tracking succeeded [Detail] [Expand]

    Breaking Changes Tracking

    ️❌ azure-sdk-for-net-track2 failed [Detail]
    • Failed [Logs]Release - Generate from 052a4b8. 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
      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] �[31;1mGeneratePackage: �[0m/mnt/vss/_work/1/s/azure-sdk-for-net/eng/scripts/automation/GenerateAndBuildLib.ps1:712
      cmderr	[Invoke-GenerateAndBuildV2.ps1] �[36;1mLine |
      cmderr	[Invoke-GenerateAndBuildV2.ps1] �[36;1m 712 | �[0m         �[36;1mGeneratePackage -projectFolder $projectFolder -sdkRootPath $s�[0m …
      cmderr	[Invoke-GenerateAndBuildV2.ps1] �[36;1m     | �[31;1m         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      cmderr	[Invoke-GenerateAndBuildV2.ps1] �[31;1m�[36;1m     | �[31;1mFailed to build sdk. exit code: False
      cmderr	[Invoke-GenerateAndBuildV2.ps1] �[0m
      cmderr	[Invoke-GenerateAndBuildV2.ps1] �[31;1mGeneratePackage: �[0m/mnt/vss/_work/1/s/azure-sdk-for-net/eng/scripts/automation/GenerateAndBuildLib.ps1:712
      cmderr	[Invoke-GenerateAndBuildV2.ps1] �[36;1mLine |
      cmderr	[Invoke-GenerateAndBuildV2.ps1] �[36;1m 712 | �[0m         �[36;1mGeneratePackage -projectFolder $projectFolder -sdkRootPath $s�[0m …
      cmderr	[Invoke-GenerateAndBuildV2.ps1] �[36;1m     | �[31;1m         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      cmderr	[Invoke-GenerateAndBuildV2.ps1] �[31;1m�[36;1m     | �[31;1mFailed to packe sdk. exit code: False
      cmderr	[Invoke-GenerateAndBuildV2.ps1] �[0m
      cmderr	[Invoke-GenerateAndBuildV2.ps1] �[31;1mGet-ChildItem: �[0m/mnt/vss/_work/1/s/azure-sdk-for-net/eng/scripts/automation/GenerateAndBuildLib.ps1:805
      cmderr	[Invoke-GenerateAndBuildV2.ps1] �[36;1mLine |
      cmderr	[Invoke-GenerateAndBuildV2.ps1] �[36;1m 805 | �[0m … rtifacts += �[36;1mGet-ChildItem $artifactsPath -Filter *.nupkg -exclude *.s�[0m …
      cmderr	[Invoke-GenerateAndBuildV2.ps1] �[36;1m     | �[31;1m               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      cmderr	[Invoke-GenerateAndBuildV2.ps1] �[31;1m�[36;1m     | �[31;1mCannot find path
      cmderr	[Invoke-GenerateAndBuildV2.ps1] �[36;1m     | �[31;1m'/mnt/vss/_work/1/s/azure-sdk-for-net/artifacts/packages/Debug/' because
      cmderr	[Invoke-GenerateAndBuildV2.ps1] �[36;1m     | �[31;1mit does not exist.
      cmderr	[Invoke-GenerateAndBuildV2.ps1] �[0m
      cmderr	[Invoke-GenerateAndBuildV2.ps1] �[31;1mGeneratePackage: �[0m/mnt/vss/_work/1/s/azure-sdk-for-net/eng/scripts/automation/GenerateAndBuildLib.ps1:712
      cmderr	[Invoke-GenerateAndBuildV2.ps1] �[36;1mLine |
      cmderr	[Invoke-GenerateAndBuildV2.ps1] �[36;1m 712 | �[0m         �[36;1mGeneratePackage -projectFolder $projectFolder -sdkRootPath $s�[0m …
      cmderr	[Invoke-GenerateAndBuildV2.ps1] �[36;1m     | �[31;1m         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      cmderr	[Invoke-GenerateAndBuildV2.ps1] �[31;1m�[36;1m     | �[31;1mFailed to generate sdk artifact
      cmderr	[Invoke-GenerateAndBuildV2.ps1] �[0m
    • Azure.Messaging.WebPubSub [View full logs]  [Release SDK Changes] Breaking Change Detected
      info	[Changelog] Breaking Changes: /home/cloudtest/.nuget/packages/microsoft.dotnet.apicompat/5.0.0-beta.20467.1/build/Microsoft.DotNet.ApiCompat.targets(82,5): error : TypesMustExist : Type 'Azure.Messaging.WebPubSub.WebPubSubServiceClient' does not exist in the implementation but it does exist in the contract. [/mnt/vss/_work/1/s/azure-sdk-for-net/sdk/webpubsub/Azure.Messaging.WebPubSub/src/Azure.Messaging.WebPubSub.csproj::TargetFramework=netstandard2.0],
      info	[Changelog] /home/cloudtest/.nuget/packages/microsoft.dotnet.apicompat/5.0.0-beta.20467.1/build/Microsoft.DotNet.ApiCompat.targets(82,5): error : TypesMustExist : Type 'Azure.Messaging.WebPubSub.WebPubSubServiceClientOptions' does not exist in the implementation but it does exist in the contract. [/mnt/vss/_work/1/s/azure-sdk-for-net/sdk/webpubsub/Azure.Messaging.WebPubSub/src/Azure.Messaging.WebPubSub.csproj::TargetFramework=netstandard2.0],
      info	[Changelog] /home/cloudtest/.nuget/packages/microsoft.dotnet.apicompat/5.0.0-beta.20467.1/build/Microsoft.DotNet.ApiCompat.targets(82,5): error : TypesMustExist : Type 'Microsoft.Extensions.Azure.WebPubSubServiceClientBuilderExtensions' does not exist in the implementation but it does exist in the contract. [/mnt/vss/_work/1/s/azure-sdk-for-net/sdk/webpubsub/Azure.Messaging.WebPubSub/src/Azure.Messaging.WebPubSub.csproj::TargetFramework=netstandard2.0],
      info	[Changelog] /home/cloudtest/.nuget/packages/microsoft.dotnet.apicompat/5.0.0-beta.20467.1/build/Microsoft.DotNet.ApiCompat.targets(96,5): error : ApiCompat failed for '/mnt/vss/_work/1/s/azure-sdk-for-net/artifacts/bin/Azure.Messaging.WebPubSub/Debug/netstandard2.0/Azure.Messaging.WebPubSub.dll' [/mnt/vss/_work/1/s/azure-sdk-for-net/sdk/webpubsub/Azure.Messaging.WebPubSub/src/Azure.Messaging.WebPubSub.csproj::TargetFramework=netstandard2.0]
    Posted by Swagger Pipeline | How to fix these errors?

    @openapi-pipeline-app
    Copy link

    openapi-pipeline-app bot commented May 11, 2023

    Generated ApiView

    Language Package Name ApiView Link
    Swagger webpubsub-data-plane-WebPubSub https://apiview.dev/Assemblies/Review/6ca31eb0866e46b7be90fd7db5848ece

    @AzureRestAPISpecReview AzureRestAPISpecReview added BreakingChangeReviewRequired <valid label in PR review process>add this label when breaking change review is required data-plane labels May 11, 2023
    @openapi-workflow-bot
    Copy link

    Hi @xingsy97, one or multiple breaking change(s) is detected in your PR. Please check out the breaking change(s), and provide business justification in the PR comment and @ PR assignee why you must have these change(s), and how external customer impact can be mitigated. Please ensure to follow breaking change policy to request breaking change review and approval before proceeding swagger PR review.
    Action: To initiate an evaluation of the breaking change, create a new intake using the template for breaking changes. Addition details on the process and office hours are on the Breaking change Wiki.
    If you want to know the production traffic statistic, please see ARM Traffic statistic.
    If you think it is false positive breaking change, please provide the reasons in the PR comment, report to Swagger Tooling Team via https://aka.ms/swaggerfeedback.
    Note: To avoid breaking change, you can refer to Shift Left Solution for detecting breaking change in early phase at your service code repository.

    @xingsy97 xingsy97 force-pushed the xingsy97-webpubsub-WebPubSub-2023-07-01 branch from d227f43 to dade725 Compare May 12, 2023 10:59
    @AzureRestAPISpecReview AzureRestAPISpecReview added BreakingChangeReviewRequired <valid label in PR review process>add this label when breaking change review is required and removed BreakingChangeReviewRequired <valid label in PR review process>add this label when breaking change review is required labels May 12, 2023
    @xingsy97 xingsy97 marked this pull request as ready for review May 15, 2023 06:04
    @xingsy97 xingsy97 requested a review from a team as a code owner May 15, 2023 06:04
    @xingsy97 xingsy97 requested review from markweitzel and johanste and removed request for a team May 15, 2023 06:04
    @xingsy97
    Copy link
    Contributor Author

    xingsy97 commented May 15, 2023

    Hi @anuchandy
    The pipeline check failed because the LRO extension "x-ms-long-running-operation": true for responses with 202 status code is required.

    However, the related APIs are designed to be "fire-and-forget". Our service is designed to not monitor the status of related API operations. Could you please help this pr pass LRO extension check? Thanks

    @AzureRestAPISpecReview AzureRestAPISpecReview added CI-FixRequiredOnFailure and removed BreakingChangeReviewRequired <valid label in PR review process>add this label when breaking change review is required labels May 18, 2023
    @openapi-workflow-bot
    Copy link

    Hi @xingsy97, Your PR has some issues. Please fix the CI sequentially by following the order of Avocado, semantic validation, model validation, breaking change, lintDiff. If you have any questions, please post your questions in this channel https://aka.ms/swaggersupport.

    TaskHow to fixPriority
    AvocadoFix-AvocadoHigh
    Semantic validationFix-SemanticValidation-ErrorHigh
    Model validationFix-ModelValidation-ErrorHigh
    LintDiffFix-LintDiffhigh
    If you need further help, please feedback via swagger feedback.

    @xingsy97
    Copy link
    Contributor Author

    Hi @markweitzel @johanste , could you please help review this PR? Our team is looking forward to see the new API design ready thus we could have plenty of time to implement it. Thanks.

    @AzureRestAPISpecReview AzureRestAPISpecReview added the BreakingChangeReviewRequired <valid label in PR review process>add this label when breaking change review is required label Jun 7, 2023
    @zackliu zackliu force-pushed the xingsy97-webpubsub-WebPubSub-2023-07-01 branch from 94ea499 to 80df848 Compare June 9, 2023 02:40
    @AzureRestAPISpecReview AzureRestAPISpecReview removed the BreakingChangeReviewRequired <valid label in PR review process>add this label when breaking change review is required label Jun 9, 2023
    @zackliu zackliu force-pushed the xingsy97-webpubsub-WebPubSub-2023-07-01 branch from 80df848 to cdcf3fb Compare June 9, 2023 02:48
    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.

    I flagged a number of issues reported by the Spectral linter. Most should be easy fixes.

    Comment on lines 46 to 51
    "consumes": [
    "application/json-patch+json",
    "application/json",
    "text/json",
    "application/*+json"
    ],
    Copy link
    Member

    Choose a reason for hiding this comment

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

    You should only allow application/json. Supported all these other content types only adds complexity and opportunity for errors in both the service implementation and generated (or otherwise) client libraries.

    Suggested change
    "consumes": [
    "application/json-patch+json",
    "application/json",
    "text/json",
    "application/*+json"
    ],
    "consumes": [
    "application/json"
    ],

    Copy link
    Contributor Author

    Choose a reason for hiding this comment

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

    updated

    Comment on lines 71 to 78
    {
    "in": "body",
    "name": "message",
    "description": "Target groups and connection filter.",
    "schema": {
    "$ref": "#/definitions/AddToGroupsRequest"
    }
    }
    Copy link
    Member

    Choose a reason for hiding this comment

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

    This request body is not marked as required: true. Is that intentional? Does the operation succeed if there is no body in the request?

    Copy link
    Contributor Author

    Choose a reason for hiding this comment

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

    Updated. Marked it into required:true

    },
    {
    "in": "body",
    "name": "message",
    Copy link
    Member

    Choose a reason for hiding this comment

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

    message is an odd name for this parameter. I think a name like "groupsToAdd" would be more descriptive.

    Copy link
    Contributor Author

    Choose a reason for hiding this comment

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

    renamed as suggestion

    ],
    "responses": {
    "200": {
    "description": "Success"
    Copy link
    Member

    Choose a reason for hiding this comment

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

    Does this response have a response body?

    Copy link
    Contributor Author

    Choose a reason for hiding this comment

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

    It doesn't have a response body

    Comment on lines 272 to 277
    "consumes": [
    "application/json-patch+json",
    "application/json",
    "text/json",
    "application/*+json"
    ],
    Copy link
    Member

    Choose a reason for hiding this comment

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

    As above, recommend this be limited to application/json.

    Suggested change
    "consumes": [
    "application/json-patch+json",
    "application/json",
    "text/json",
    "application/*+json"
    ],
    "consumes": [
    "application/json"
    ],

    Copy link
    Contributor Author

    Choose a reason for hiding this comment

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

    updated as suggestion

    "description": "The request object containing targets groups and connection filter",
    "type": "object",
    "properties": {
    "groups": {
    Copy link
    Member

    Choose a reason for hiding this comment

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

    Please add a description.

    Copy link
    Contributor Author

    Choose a reason for hiding this comment

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

    updated as suggestion

    }
    },
    "connectionFilter": {
    "type": "string"
    Copy link
    Member

    Choose a reason for hiding this comment

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

    Please add a description.

    Copy link
    Contributor Author

    Choose a reason for hiding this comment

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

    updated as suggestion

    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.

    Thanks for all the fixes. Looks good! 👍

    @mikekistler mikekistler added the APIStewardshipBoard-SignedOff The Azure API Stewardship team has reviewed and approved the changes. label Jun 16, 2023
    Copy link
    Member

    @weidongxu-microsoft weidongxu-microsoft left a comment

    Choose a reason for hiding this comment

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

    LGTM.

    But see if you can improve the description on filter.

    @anuchandy anuchandy merged commit 052a4b8 into main Jun 16, 2023
    @anuchandy anuchandy deleted the xingsy97-webpubsub-WebPubSub-2023-07-01 branch June 16, 2023 15:29
    @mikekistler mikekistler added the Web PubSub Web PubSub service label Jun 18, 2023
    harryli0108 pushed a commit to harryli0108/azure-rest-api-specs that referenced this pull request Jul 28, 2023
    …23-07-01 (Azure#23927)
    
    * Original
    
    * Add 2023-07-01
    
    * update
    
    * update
    
    * remove invoke
    
    * resolve comments
    
    * resolve comments
    
    * resolve failed check
    
    * resolve failed check
    
    ---------
    
    Co-authored-by: Chenyang Liu <chenyl@microsoft.com>
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    APIStewardshipBoard-SignedOff The Azure API Stewardship team has reviewed and approved the changes. data-plane Web PubSub Web PubSub service
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    [Azure SignalR Service - Azure Web PubSub] API Review
    7 participants