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

[Communication] Adding 2021-06-21 TURN API version #14955

Merged
merged 1 commit into from
Jun 28, 2021
Merged

[Communication] Adding 2021-06-21 TURN API version #14955

merged 1 commit into from
Jun 28, 2021

Conversation

ajpeacock0
Copy link
Contributor

Adding a new TURN/NetworkTraversal REST API version. Changes include

  • Move the identity ({id}) from the path to the body or query parameter. Since the convention across ACS is to have it in the body, it should be in the body.
  • Rename CommunicationTurnServer to CommunicationIceServer
  • Rename CommunicationTurnCredentialsResponse to CommunicationRelayConfiguration
  • Rename the service name path from /turn to something more generic to allow future paths (e.g. /networktraversal). This name (networktraversal) is not final and should be discussed offline for agreement
  • Rename /:issueCredentials to /:issueRelayConfiguration
  • Ensure the format of expiresOn date-time is rfc 3339 (if it is the same as all other ACS date-times)
  • Remove the trailing number (e.g. 1) from the preview version (e.g. 2021-06-21-preview1 -> 2021-06-21-preview)

Changelog

Please ensure to add changelog with this PR by answering the following questions.

  1. What's the purpose of the update?
    • new service onboarding
    • [X ] new API version
    • update existing version for new feature
    • update existing version to fix swagger quality issue in s360
    • Other, please clarify
  2. When you are targeting to deploy new service/feature to public regions? Please provide date, or month to public if date is not available yet.
  3. When you expect to publish swagger? Please provide date, or month to public if date is not available yet.

Contribution checklist:

Please follow the link to find more details on PR review process.

@openapi-workflow-bot
Copy link

Hi, @ajpeacock0 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. vsswagger@microsoft.com

    @openapi-workflow-bot
    Copy link

    [Call for Action] To better understand Azure service dev/test scenario, and support Azure service developer better on Swagger and REST API related tests in early phase, please help to fill in with this survey https://aka.ms/SurveyForEarlyPhase. It will take 5 to 10 minutes. If you already complete survey, please neglect this comment. Thanks.

    @openapi-pipeline-app
    Copy link

    openapi-pipeline-app bot commented Jun 23, 2021 •
    edited
    Loading

    Swagger Validation Report

    ️❌BreakingChange: 2 Errors, 0 Warnings failed [Detail]
    Rule Message
    ❌ 1016 - ConstantStatusHasChanged The 'constant' status changed from the old version to the new.
    New: Turn/preview/2021-02-22-preview1/CommunicationTurn.json#L19:9
    Old: Turn/preview/2021-02-22-preview1/CommunicationTurn.json#L19:9
    ❌ 1016 - ConstantStatusHasChanged The 'constant' status changed from the old version to the new.
    New: Turn/preview/2021-02-22-preview1/CommunicationTurn.json#L104:5
    Old: Turn/preview/2021-02-22-preview1/CommunicationTurn.json#L104:5
    ️⚠️LintDiff: 1 Warnings warning [Detail]
    The following errors/warnings are introduced by current PR:
    Rule Message
    ⚠️ R4000 - ParameterDescriptionRequired 'body' parameter lacks 'description' property. Consider adding a 'description' element. Accurate description is essential for maintaining reference documentation.
    Location: Turn/preview/2021-06-21-preview/CommunicationTurn.json#L26
    ️️✔️Avocado succeeded [Detail] [Expand] 
    Validation passes for Avocado.
    ️️✔️ModelValidation succeeded [Detail] [Expand] 
    Validation passes for ModelValidation.
    ️️✔️SemanticValidation succeeded [Detail] [Expand] 
    Validation passes for SemanticValidation.
    ️❌Cross-Version Breaking Changes: 4 Errors, 1 Warnings failed [Detail]
    The following breaking changes are detected by comparison with latest preview version:
    Rule Message
    ❌ 1005 - RemovedPath The new version is missing a path that was found in the old version. Was path '/turn/{id}/:issueCredentials' removed or restructured?
    Old: Turn/preview/2021-02-22-preview1/CommunicationTurn.json#L9:5
    ❌ 1033 - RemovedProperty The new version is missing a property found in the old version. Was 'turnServers' renamed or removed?
    New: Turn/preview/2021-06-21-preview/CommunicationTurn.json#L104:7
    Old: Turn/preview/2021-02-22-preview1/CommunicationTurn.json#L87:7
    ❌ 1034 - AddedRequiredProperty The new version has new required property 'iceServers' that was not found in the old version.
    New: Turn/preview/2021-06-21-preview/CommunicationTurn.json#L104:7
    Old: Turn/preview/2021-02-22-preview1/CommunicationTurn.json#L87:7
    ❌ 1034 - AddedRequiredProperty The new version has new required property 'iceServers' that was not found in the old version.
    New: Turn/preview/2021-06-21-preview/CommunicationTurn.json#L97:5
    Old: Turn/preview/2021-02-22-preview1/CommunicationTurn.json#L80:5
    ⚠️ 1016 - ConstantStatusHasChanged The 'constant' status changed from the old version to the new.
    New: Turn/preview/2021-06-21-preview/CommunicationTurn.json#L121:5
    Old: Turn/preview/2021-02-22-preview1/CommunicationTurn.json#L104:5
    ️️✔️CredScan succeeded [Detail] [Expand] 
    There is no credential detected.
    ️️✔️[Staging] SDK Track2 Validation succeeded [Detail] [Expand] 
    Validation passes for SDKTrack2Validation

    The following errors/warnings are introduced by current PR:

    |:speech_balloon: AutorestCore/Exception|"readme":"communication/data-plane/Turn/readme.md",
    "tag":"package-2021-02-22-preview1",
    "details":"> Installing AutoRest extension '@microsoft.azure/openapi-validator' (1.8.0)"|
    |:speech_balloon: AutorestCore/Exception|"readme":"communication/data-plane/Turn/readme.md",
    "tag":"package-2021-02-22-preview1",
    "details":"> Installed AutoRest extension '@microsoft.azure/openapi-validator' (1.8.0->1.8.0)"|
    |:speech_balloon: AutorestCore/Exception|"readme":"communication/data-plane/Turn/readme.md",
    "tag":"package-2021-06-21-preview",
    "details":"> Loading AutoRest extension '@microsoft.azure/openapi-validator' (1.8.0->1.8.0)"|
    |:speech_balloon: AutorestCore/Exception|"readme":"communication/data-plane/Turn/readme.md",
    "tag":"package-2021-06-21-preview",
    "details":"> Loading AutoRest extension '@autorest/modelerfour' (4.15.456->4.15.456)"|


    The following errors/warnings exist before current PR submission:

    |:speech_balloon: AutorestCore/Exception|"readme":"communication/data-plane/Turn/readme.md",
    "tag":"package-2021-02-22-preview1",
    "details":"> Loading AutoRest extension '@autorest/modelerfour' (4.15.456->4.15.456)"|

    ️️✔️[Staging] PrettierCheck succeeded [Detail] [Expand] 
    Validation passes for PrettierCheck.
    ️️✔️[Staging] SpellCheck succeeded [Detail] [Expand] 
    Validation passes for SpellCheck.
    ️️✔️[Staging] Lint(RPaaS) succeeded [Detail] [Expand] 
    Validation passes for Lint(RPaaS).
    Posted by Swagger Pipeline | How to fix these errors?

    @openapi-pipeline-app
    Copy link

    openapi-pipeline-app bot commented Jun 23, 2021 •
    edited
    Loading

    Swagger pipeline restarted successfully, please wait for status update in this comment.

    @openapi-workflow-bot
    Copy link

    Hi @ajpeacock0, 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.

    @anuchandy
    Copy link
    Member

    Hi @JeffreyRichter, this preview version of service has breaking changes compared to the previous preview apiVersion, the pr renames the endpoint from turn to networktraversal based on the feedback from the Azure REST review.

    @JeffreyRichter JeffreyRichter added the Approved-BreakingChange DO NOT USE! OBSOLETE label. See https://github.com/Azure/azure-sdk-tools/issues/6374 label Jun 28, 2021
    @anuchandy anuchandy merged commit 86408a8 into Azure:master Jun 28, 2021
    mkarmark pushed a commit to mkarmark/azure-rest-api-specs that referenced this pull request Jul 21, 2021
    @ajpeacock0 @mkarmark
    Adding 2021-06-21 TURN API version (Azure#14955)
    33da7fe
    @ajpeacock0 ajpeacock0 mentioned this pull request Feb 2, 2022
    Merged
    12 tasks
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Reviewers

    @anuchandy anuchandy anuchandy approved these changes

    @JeffreyRichter JeffreyRichter Awaiting requested review from JeffreyRichter

    Assignees

    @anuchandy anuchandy

    Labels
    Approved-BreakingChange DO NOT USE! OBSOLETE label. See https://github.com/Azure/azure-sdk-tools/issues/6374
    Projects
    None yet
    Milestone
    No milestone
    Development

    Successfully merging this pull request may close these issues.
    3 participants
    @ajpeacock0 @anuchandy @JeffreyRichter