azure-rest-api-specs icon indicating copy to clipboard operation
azure-rest-api-specs copied to clipboard
verified publisher icon Azure

Adding Path Object to Typespec for CLU

Open Zach-King2 opened this issue 2 years ago • 11 comments

Zach-King2 avatar Jun 26 '23 23:06 Zach-King2

Hi, @Zach-King2 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. [email protected]

    openapi-workflow-bot[bot] avatar Jun 26 '23 23:06 openapi-workflow-bot[bot]

    Swagger Validation Report

    ️❌BreakingChange: 1 Errors, 0 Warnings failed [Detail]
    Rule Message
    ❌ Runtime Exception "new":"https://github.com/Azure/azure-rest-api-specs/blob/e274e544fd440ca34bec9c913d3b4a049b346280/specification/cognitiveservices/data-plane/Language/stable/2022-05-01/analyzeconversations.json",
    "old":"https://github.com/Azure/azure-rest-api-specs/blob/main/specification/cognitiveservices/data-plane/Language/stable/2022-05-01/analyzeconversations.json",
    "details":"Command failed: node "/mnt/vss/_work/_tasks/AzureApiValidation_5654d05d-82c1-48da-ad8f-161b817f6d41/0.0.54/common/temp/node_modules/.pnpm/@[email protected]/node_modules/autorest/dist/app.js" --v2 --input-file=specification/cognitiveservices/data-plane/Language/stable/2022-05-01/analyzeconversations.json --output-artifact=swagger-document.json --output-artifact=swagger-document.map --output-file=new --output-folder=/tmp\nERROR: Schema violation: Missing required property: name\n - file:///mnt/vss/work/1/azure-rest-api-specs/specification/cognitiveservices/data-plane/Language/stable/2022-05-01/analyzeconversations.json:2090:10 ($.definitions.apimkey.properties.name["x-ms-enum"])\nFATAL: swagger-document/individual/schema-validator - FAILED\nFATAL: Error: [OperationAbortedException] Error occurred. Exiting.\nProcess() cancelled due to exception : [OperationAbortedException] Error occurred. Exiting.\n"
    ️️✔️Breaking Change(Cross-Version) succeeded [Detail] [Expand] 
    There are no breaking changes.
    ️⚠️LintDiff: 17 Warnings warning [Detail]
    compared tags (via openapi-validator v2.1.3) new version base version
    default default(e274e54) default(feature-cognitiveservices-Language.Conversations-typespec)
    release_2022_05_01 release_2022_05_01(e274e54) release_2022_05_01(feature-cognitiveservices-Language.Conversations-typespec)

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

    Rule Message Related RPC [For API reviewers]
    :warning: XmsExamplesRequired Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations.
    Location: Language/stable/2022-05-01/analyzeconversations.json#L50
    :warning: OperationId OperationId should be of the form 'Noun_Verb'
    Location: Language/stable/2022-05-01/analyzeconversations.json#L51
    :warning: ParameterDescription Parameter should have a description.
    Location: Language/stable/2022-05-01/analyzeconversations.json#L57
    :warning: SchemaNamesConvention Schema name should be Pascal case.
    Location: Language/stable/2022-05-01/analyzeconversations.json#L90
    :warning: 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: Language/stable/2022-05-01/analyzeconversations.json#L319
    :warning: SchemaNamesConvention Schema name should be Pascal case.
    Location: Language/stable/2022-05-01/analyzeconversations.json#L387
    :warning: SchemaNamesConvention Schema name should be Pascal case.
    Location: Language/stable/2022-05-01/analyzeconversations.json#L422
    :warning: SchemaNamesConvention Schema name should be Pascal case.
    Location: Language/stable/2022-05-01/analyzeconversations.json#L435
    :warning: AdditionalPropertiesAndProperties Don't specify additionalProperties as a sibling of properties.
    Location: Language/stable/2022-05-01/analyzeconversations.json#L1087
    :warning: 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: Language/stable/2022-05-01/analyzeconversations.json#L1156
    :warning: 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: Language/stable/2022-05-01/analyzeconversations.json#L1671
    :warning: SchemaNamesConvention Schema name should be Pascal case.
    Location: Language/stable/2022-05-01/analyzeconversations.json#L2068
    :warning: SchemaNamesConvention Schema name should be Pascal case.
    Location: Language/stable/2022-05-01/analyzeconversations.json#L2103
    :warning: SchemaNamesConvention Schema name should be Pascal case.
    Location: Language/stable/2022-05-01/analyzeconversations.json#L2115
    :warning: SchemaNamesConvention Schema name should be Pascal case.
    Location: Language/stable/2022-05-01/analyzeconversations.json#L2120
    :warning: SchemaNamesConvention Schema name should be Pascal case.
    Location: Language/stable/2022-05-01/analyzeconversations.json#L2132
    :warning: SchemaNamesConvention Schema name should be Pascal case.
    Location: Language/stable/2022-05-01/analyzeconversations.json#L2157


    The following errors/warnings exist before current PR submission:

    Rule Message
    :warning: 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: Language/stable/2022-05-01/analyzeconversations.json#L498
    :warning: AdditionalPropertiesAndProperties Don't specify additionalProperties as a sibling of properties.
    Location: Language/stable/2022-05-01/analyzeconversations.json#L516
    :warning: 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: Language/stable/2022-05-01/analyzeconversations.json#L533
    :warning: 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: Language/stable/2022-05-01/analyzeconversations.json#L537
    :warning: AdditionalPropertiesAndProperties Don't specify additionalProperties as a sibling of properties.
    Location: Language/stable/2022-05-01/analyzeconversations.json#L653
    :warning: 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: Language/stable/2022-05-01/analyzeconversations.json#L753
    :warning: 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: Language/stable/2022-05-01/analyzeconversations.json#L757
    :warning: AdditionalPropertiesAndProperties Don't specify additionalProperties as a sibling of properties.
    Location: Language/stable/2022-05-01/analyzeconversations.json#L787
    :warning: 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: Language/stable/2022-05-01/analyzeconversations.json#L1283
    :warning: 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: Language/stable/2022-05-01/analyzeconversations.json#L1287
    :warning: 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: Language/stable/2022-05-01/analyzeconversations.json#L1291
    :warning: 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: Language/stable/2022-05-01/analyzeconversations.json#L1301
    :warning: AdditionalPropertiesAndProperties Don't specify additionalProperties as a sibling of properties.
    Location: Language/stable/2022-05-01/analyzeconversations.json#L1345
    :warning: AdditionalPropertiesAndProperties Don't specify additionalProperties as a sibling of properties.
    Location: Language/stable/2022-05-01/analyzeconversations.json#L1928
    ️❌Avocado: 3 Errors, 0 Warnings failed [Detail]
    Rule Message
    ❌ UNREFERENCED_JSON_FILE The example JSON file is not referenced from the swagger file.
    readme: cognitiveservices/data-plane/Language/readme.md
    json: 2022-05-01/examples/conversations/SuccessfulAnalyzeConversations.json
    ❌ UNREFERENCED_JSON_FILE The example JSON file is not referenced from the swagger file.
    readme: cognitiveservices/data-plane/Language/readme.md
    json: 2022-05-01/examples/conversations/SuccessfulAnalyzeConversationsArbitration.json
    ❌ UNREFERENCED_JSON_FILE The example JSON file is not referenced from the swagger file.
    readme: cognitiveservices/data-plane/Language/readme.md
    json: 2022-05-01/examples/conversations/SuccessfulAnalyzeConversationsArbitrationDirectTarget.json
    ️️✔️SwaggerAPIView succeeded [Detail] [Expand]
    ️️✔️CadlAPIView succeeded [Detail] [Expand]
    ️❌TypeSpecAPIView: 0 Errors, 1 Warnings failed [Detail]
    Rule Message
    :warning: Failed to generate TypeSpec APIView. Please check the detail log and make sure TypeSpec compiler version is the latest. "How to fix":"Check the detailed log and verify if the TypeSpec emitter is able to create API review file for the changes in PR."
    ️❌ModelValidation: 1 Errors, 0 Warnings failed [Detail]
    Rule Message
    ❌ XMS_EXAMPLE_NOTFOUND_ERROR x-ms-example not found in AnalyzeConversation.
    Url: Language/stable/2022-05-01/analyzeconversations.json#L50:15
    ️️✔️SemanticValidation succeeded [Detail] [Expand] 
    Validation passes for SemanticValidation.
    ️️✔️PrettierCheck succeeded [Detail] [Expand] 
    Validation passes for PrettierCheck.
    ️️✔️SpellCheck succeeded [Detail] [Expand] 
    Validation passes for SpellCheck.
    ️️✔️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.
    ️⌛Please ignore, experimental check pending [Detail]
    Posted by Swagger Pipeline | How to fix these errors?

    openapi-pipeline-app[bot] avatar Jun 26 '23 23:06 openapi-pipeline-app[bot]

    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 warning [Detail]
    • ⚠️Warning [Logs] Generate from f743c22df73a138ef3614aa0700c38bb1f587b68. 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/cognitiveservices/Language.Conversations/tspconfig.yaml skipped due to azure-sdk-for-net-track2 not found in tspconfig.yaml
command	pwsh ./eng/scripts/Invoke-GenerateAndBuildV2.ps1 ../azure-sdk-for-net_tmp/generateInput.json ../azure-sdk-for-net_tmp/generateOutput.json
warn	No file changes detected after generation
warn	Skip detect changed packages
    Posted by Swagger Pipeline | How to fix these errors?

    openapi-pipeline-app[bot] avatar Jun 26 '23 23:06 openapi-pipeline-app[bot]

    Generated ApiView

    Language Package Name ApiView Link
    Swagger cognitiveservices-data-plane-Language https://apiview.dev/Assemblies/Review/2a75662cb41743718001f473da43cba6

    openapi-pipeline-app[bot] avatar Jun 26 '23 23:06 openapi-pipeline-app[bot]

    Hi @Zach-King2! Your PR has some issues. Please fix the CI issues, if present, in following order: Avocado, SemanticValidation, ModelValidation, Breaking Change, LintDiff.

    TaskHow to fixPriority
    AvocadoFix-AvocadoHigh
    Semantic ValidationFix-SemanticValidation-ErrorHigh
    Model ValidationFix-ModelValidation-ErrorHigh
    LintDiffFix-LintDiffHigh

    If you need further help, please reach out on the Teams channel aka.ms/azsdk/support/specreview-channel.

    openapi-workflow-bot[bot] avatar Jun 26 '23 23:06 openapi-workflow-bot[bot]

    @Zach-King2 go through the error and a couple warnings in https://dev.azure.com/azure-sdk/internal/_build/results?buildId=2894774&view=logs&j=e37a1bb9-a190-5269-3a0e-37469dfbe682&t=09cb0672-20c5-5dce-4552-dad3f3e96fd3 and see if you can resolve those. For the missing file under eng/, just copy it. Assuming no changes when the feature branch is merged, it won't make a difference.

    heaths avatar Jul 05 '23 21:07 heaths

    @Zach-King2 go through the error and a couple warnings in https://dev.azure.com/azure-sdk/internal/_build/results?buildId=2894774&view=logs&j=e37a1bb9-a190-5269-3a0e-37469dfbe682&t=09cb0672-20c5-5dce-4552-dad3f3e96fd3 and see if you can resolve those. For the missing file under eng/, just copy it. Assuming no changes when the feature branch is merged, it won't make a difference.

    I took a look and the tsconfig.json file is in eng/tools/TypeSpecValidation. I'm not sure why its not being recognized, I didn't make any changes in this folder.

    Zach-King2 avatar Jul 05 '23 22:07 Zach-King2

    @Zach-King2 go through the error and a couple warnings in https://dev.azure.com/azure-sdk/internal/_build/results?buildId=2894774&view=logs&j=e37a1bb9-a190-5269-3a0e-37469dfbe682&t=09cb0672-20c5-5dce-4552-dad3f3e96fd3 and see if you can resolve those. For the missing file under eng/, just copy it. Assuming no changes when the feature branch is merged, it won't make a difference.

    I took a look and the tsconfig.json file is in eng/tools/TypeSpecValidation. I'm not sure why its not being recognized, I didn't make any changes in this folder.

    @heaths Could this be related to the compiler issue?

    Zach-King2 avatar Jul 05 '23 22:07 Zach-King2

    @heaths Could this be related to the compiler issue?

    Not sure. @allenjzhang can someone take a look? It seems files are up to date with what's in main. Long-term, we need a solution that works across feature branches since we want - need - teams to be using feature branches until they are ready to publish. Constantly having to merge main is going to be problematic.

    heaths avatar Jul 06 '23 17:07 heaths

    Please see the comments above to fix the item. They should get you green on the TypeSpec validation & Model validation errors. @mikeharder, the APIView step is failing. Is it there a pipeline fix?

    Oh, also more linting rules RESTApi board asked have been added. Those warnings require you to add more @doc or /** */ to many of the enum/properties. Please fix all warnings as well. Thx

    allenjzhang avatar Jul 06 '23 23:07 allenjzhang

    @allenjzhang thank you. The PR is incomplete e.g., comments, because this is very much a work in progress; though, I agree something should be added as much as possible to make this green. @Zach-King2 sounds like adding comments as you author models would be best. You can just copy/paste, of course.

    heaths avatar Jul 06 '23 23:07 heaths

    Some failures are expected for now. @Zach-King2 will start a new PR to fill out most/all the remaining models that should fix almost all the remaining issues that aren't tooling-related.

    heaths avatar Jul 14 '23 17:07 heaths