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

Rename Language 2023-05-01 to 2023-04-01 #21990

Merged
merged 3 commits into from
Jan 19, 2023

Conversation

AhmedLeithy
Copy link
Member

No description provided.

@AhmedLeithy AhmedLeithy requested review from JeffreyRichter and johanste and removed request for a team December 26, 2022 12:39
@openapi-workflow-bot
Copy link

Hi, @AhmedLeithy 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-pipeline-app
    Copy link

    openapi-pipeline-app bot commented Dec 26, 2022

    Swagger Validation Report

    ️️✔️BreakingChange succeeded [Detail] [Expand]
    There are no breaking changes.
    compared swaggers (via Oad v0.10.4)] new version base version
    analyzeconversations-authoring.json 2023-04-01(ebbacbe) 2023-05-01(main)
    analyzeconversations.json 2023-04-01(ebbacbe) 2023-05-01(main)
    analyzetext-authoring.json 2023-04-01(ebbacbe) 2023-05-01(main)
    analyzetext.json 2023-04-01(ebbacbe) 2023-05-01(main)
    common.json 2023-04-01(ebbacbe) 2023-05-01(main)
    questionanswering-authoring.json 2023-04-01(ebbacbe) 2023-05-01(main)
    questionanswering.json 2023-04-01(ebbacbe) 2023-05-01(main)
    ️❌Breaking Change(Cross-Version): 41 Errors, 38 Warnings failed [Detail]
    compared swaggers (via Oad v0.10.4)] new version base version
    analyzeconversations-authoring.json 2023-04-01(ebbacbe) 2022-05-01(main)
    analyzeconversations-authoring.json 2023-04-01(ebbacbe) 2022-10-01-preview(main)
    analyzeconversations.json 2023-04-01(ebbacbe) 2022-05-01(main)
    analyzeconversations.json 2023-04-01(ebbacbe) 2022-10-01-preview(main)
    analyzetext-authoring.json 2023-04-01(ebbacbe) 2022-05-01(main)
    analyzetext-authoring.json 2023-04-01(ebbacbe) 2022-10-01-preview(main)
    analyzetext.json 2023-04-01(ebbacbe) 2022-05-01(main)
    analyzetext.json 2023-04-01(ebbacbe) 2022-10-01-preview(main)
    common.json 2023-04-01(ebbacbe) 2022-05-01(main)
    common.json 2023-04-01(ebbacbe) 2022-10-01-preview(main)
    questionanswering-authoring.json 2023-04-01(ebbacbe) 2021-10-01(main)
    questionanswering-authoring.json 2023-04-01(ebbacbe) 2022-10-01-preview(main)
    questionanswering.json 2023-04-01(ebbacbe) 2021-10-01(main)
    questionanswering.json 2023-04-01(ebbacbe) 2022-10-01-preview(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 '/query-knowledgebases/projects' removed or restructured?
    Old: Language/stable/2021-10-01/questionanswering-authoring.json#L30:5
    1005 - RemovedPath The new version is missing a path that was found in the old version. Was path '/query-knowledgebases/projects/{projectName}' removed or restructured?
    Old: Language/stable/2021-10-01/questionanswering-authoring.json#L79:5
    1005 - RemovedPath The new version is missing a path that was found in the old version. Was path '/query-knowledgebases/projects/deletion-jobs/{jobId}' removed or restructured?
    Old: Language/stable/2021-10-01/questionanswering-authoring.json#L210:5
    1005 - RemovedPath The new version is missing a path that was found in the old version. Was path '/query-knowledgebases/projects/{projectName}/:export' removed or restructured?
    Old: Language/stable/2021-10-01/questionanswering-authoring.json#L249:5
    1005 - RemovedPath The new version is missing a path that was found in the old version. Was path '/query-knowledgebases/projects/{projectName}/export/jobs/{jobId}' removed or restructured?
    Old: Language/stable/2021-10-01/questionanswering-authoring.json#L303:5
    1005 - RemovedPath The new version is missing a path that was found in the old version. Was path '/query-knowledgebases/projects/{projectName}/:import' removed or restructured?
    Old: Language/stable/2021-10-01/questionanswering-authoring.json#L345:5
    1005 - RemovedPath The new version is missing a path that was found in the old version. Was path '/query-knowledgebases/projects/{projectName}/import/jobs/{jobId}' removed or restructured?
    Old: Language/stable/2021-10-01/questionanswering-authoring.json#L401:5
    1005 - RemovedPath The new version is missing a path that was found in the old version. Was path '/query-knowledgebases/projects/{projectName}/deployments/{deploymentName}' removed or restructured?
    Old: Language/stable/2021-10-01/questionanswering-authoring.json#L443:5
    1005 - RemovedPath The new version is missing a path that was found in the old version. Was path '/query-knowledgebases/projects/{projectName}/deployments/{deploymentName}/jobs/{jobId}' removed or restructured?
    Old: Language/stable/2021-10-01/questionanswering-authoring.json#L488:5
    1005 - RemovedPath The new version is missing a path that was found in the old version. Was path '/query-knowledgebases/projects/{projectName}/deployments' removed or restructured?
    Old: Language/stable/2021-10-01/questionanswering-authoring.json#L533:5
    1005 - RemovedPath The new version is missing a path that was found in the old version. Was path '/query-knowledgebases/projects/{projectName}/synonyms' removed or restructured?
    Old: Language/stable/2021-10-01/questionanswering-authoring.json#L585:5
    1005 - RemovedPath The new version is missing a path that was found in the old version. Was path '/query-knowledgebases/projects/{projectName}/sources' removed or restructured?
    Old: Language/stable/2021-10-01/questionanswering-authoring.json#L680:5
    1005 - RemovedPath The new version is missing a path that was found in the old version. Was path '/query-knowledgebases/projects/{projectName}/sources/jobs/{jobId}' removed or restructured?
    Old: Language/stable/2021-10-01/questionanswering-authoring.json#L781:5
    1005 - RemovedPath The new version is missing a path that was found in the old version. Was path '/query-knowledgebases/projects/{projectName}/qnas' removed or restructured?
    Old: Language/stable/2021-10-01/questionanswering-authoring.json#L823:5
    1005 - RemovedPath The new version is missing a path that was found in the old version. Was path '/query-knowledgebases/projects/{projectName}/qnas/jobs/{jobId}' removed or restructured?
    Old: Language/stable/2021-10-01/questionanswering-authoring.json#L927:5
    1005 - RemovedPath The new version is missing a path that was found in the old version. Was path '/query-knowledgebases/projects/{projectName}/feedback' removed or restructured?
    Old: Language/stable/2021-10-01/questionanswering-authoring.json#L969:5
    1007 - RemovedClientParameter The new version is missing a client parameter that was found in the old version. Was 'ConversationalAnalysisAuthoringFormatQueryParameter' removed or renamed?
    New: cognitiveservices/data-plane/Language/analyzeconversations-authoring.json#L3831:3
    Old: Language/stable/2022-05-01/analyzeconversations-authoring.json#L3012:3
    1025 - RequiredStatusChange The 'required' status changed from the old version('False') to the new version('True').
    New: cognitiveservices/data-plane/Language/common.json#L14:7
    Old: Language/stable/2021-10-01/common.json#L14:7
    1025 - RequiredStatusChange The 'required' status changed from the old version('False') to the new version('True').
    New: cognitiveservices/data-plane/Language/common.json#L14:7
    Old: Language/stable/2021-10-01/common.json#L14:7
    1034 - AddedRequiredProperty The new version has new required property 'entities' that was not found in the old version.
    1034 - AddedRequiredProperty The new version has new required property 'relations' that was not found in the old version.
    1034 - AddedRequiredProperty The new version has new required property 'id' that was not found in the old version.
    1034 - AddedRequiredProperty The new version has new required property 'warnings' that was not found in the old version.
    1034 - AddedRequiredProperty The new version has new required property 'entities, relations, id, warnings' that was not found in the old version.
    New: cognitiveservices/data-plane/Language/analyzetext.json#L853:11
    Old: Language/stable/2022-05-01/analyzetext.json#L779:11
    1034 - AddedRequiredProperty The new version has new required property 'entities' that was not found in the old version.
    1034 - AddedRequiredProperty The new version has new required property 'id' that was not found in the old version.
    1034 - AddedRequiredProperty The new version has new required property 'warnings' that was not found in the old version.
    1034 - AddedRequiredProperty The new version has new required property 'entities, id, warnings' that was not found in the old version.
    New: cognitiveservices/data-plane/Language/analyzetext.json#L1524:11
    Old: Language/stable/2022-05-01/analyzetext.json#L1430:11
    1034 - AddedRequiredProperty The new version has new required property 'entities' that was not found in the old version.
    1034 - AddedRequiredProperty The new version has new required property 'id' that was not found in the old version.


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

    Rule Message
    ⚠️ 1005 - RemovedPath The new version is missing a path that was found in the old version. Was path '/query-knowledgebases/projects' removed or restructured?
    Old: Language/preview/2022-10-01-preview/questionanswering-authoring.json#L45:5
    ⚠️ 1005 - RemovedPath The new version is missing a path that was found in the old version. Was path '/query-knowledgebases/projects/{projectName}' removed or restructured?
    Old: Language/preview/2022-10-01-preview/questionanswering-authoring.json#L94:5
    ⚠️ 1005 - RemovedPath The new version is missing a path that was found in the old version. Was path '/query-knowledgebases/projects/deletion-jobs/{jobId}' removed or restructured?
    Old: Language/preview/2022-10-01-preview/questionanswering-authoring.json#L225:5
    ⚠️ 1005 - RemovedPath The new version is missing a path that was found in the old version. Was path '/query-knowledgebases/projects/{projectName}/:export' removed or restructured?
    Old: Language/preview/2022-10-01-preview/questionanswering-authoring.json#L264:5
    ⚠️ 1005 - RemovedPath The new version is missing a path that was found in the old version. Was path '/query-knowledgebases/projects/{projectName}/export/jobs/{jobId}' removed or restructured?
    Old: Language/preview/2022-10-01-preview/questionanswering-authoring.json#L318:5
    ⚠️ 1005 - RemovedPath The new version is missing a path that was found in the old version. Was path '/query-knowledgebases/projects/{projectName}/:import' removed or restructured?
    Old: Language/preview/2022-10-01-preview/questionanswering-authoring.json#L360:5
    ⚠️ 1005 - RemovedPath The new version is missing a path that was found in the old version. Was path '/query-knowledgebases/projects/{projectName}/import/jobs/{jobId}' removed or restructured?
    Old: Language/preview/2022-10-01-preview/questionanswering-authoring.json#L416:5
    ⚠️ 1005 - RemovedPath The new version is missing a path that was found in the old version. Was path '/query-knowledgebases/projects/{projectName}/deployments/{deploymentName}' removed or restructured?
    Old: Language/preview/2022-10-01-preview/questionanswering-authoring.json#L458:5
    ⚠️ 1005 - RemovedPath The new version is missing a path that was found in the old version. Was path '/query-knowledgebases/projects/{projectName}/deployments/{deploymentName}/jobs/{jobId}' removed or restructured?
    Old: Language/preview/2022-10-01-preview/questionanswering-authoring.json#L503:5
    ⚠️ 1005 - RemovedPath The new version is missing a path that was found in the old version. Was path '/query-knowledgebases/projects/{projectName}/deployments' removed or restructured?
    Old: Language/preview/2022-10-01-preview/questionanswering-authoring.json#L548:5
    ⚠️ 1005 - RemovedPath The new version is missing a path that was found in the old version. Was path '/query-knowledgebases/projects/{projectName}/synonyms' removed or restructured?
    Old: Language/preview/2022-10-01-preview/questionanswering-authoring.json#L600:5
    ⚠️ 1005 - RemovedPath The new version is missing a path that was found in the old version. Was path '/query-knowledgebases/projects/{projectName}/sources' removed or restructured?
    Old: Language/preview/2022-10-01-preview/questionanswering-authoring.json#L695:5
    ⚠️ 1005 - RemovedPath The new version is missing a path that was found in the old version. Was path '/query-knowledgebases/projects/{projectName}/sources/jobs/{jobId}' removed or restructured?
    Old: Language/preview/2022-10-01-preview/questionanswering-authoring.json#L796:5
    ⚠️ 1005 - RemovedPath The new version is missing a path that was found in the old version. Was path '/query-knowledgebases/projects/{projectName}/qnas' removed or restructured?
    Old: Language/preview/2022-10-01-preview/questionanswering-authoring.json#L838:5
    ⚠️ 1005 - RemovedPath The new version is missing a path that was found in the old version. Was path '/query-knowledgebases/projects/{projectName}/qnas/jobs/{jobId}' removed or restructured?
    Old: Language/preview/2022-10-01-preview/questionanswering-authoring.json#L942:5
    ⚠️ 1005 - RemovedPath The new version is missing a path that was found in the old version. Was path '/query-knowledgebases/projects/{projectName}/feedback' removed or restructured?
    Old: Language/preview/2022-10-01-preview/questionanswering-authoring.json#L984:5
    ️️✔️CredScan succeeded [Detail] [Expand]
    There is no credential detected.
    ️🔄LintDiff inProgress [Detail]
    ️️✔️Avocado succeeded [Detail] [Expand]
    Validation passes for Avocado.
    ️️✔️ApiReadinessCheck succeeded [Detail] [Expand]
    ️️✔️~[Staging] ServiceAPIReadinessTest succeeded [Detail] [Expand]
    Validation passes for ServiceAPIReadinessTest.
    ️️✔️~[Staging] SwaggerAPIView succeeded [Detail] [Expand]
    ️️✔️~[Staging] CadlAPIView 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.
    ️️✔️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 Dec 26, 2022

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

    @openapi-pipeline-app
    Copy link

    openapi-pipeline-app bot commented Dec 26, 2022

    Swagger pipeline restarted successfully. If there is ApiView generated, it will be updated in this comment.

    @ghost ghost added the Cognitive Services label Dec 26, 2022
    @AzureRestAPISpecReview AzureRestAPISpecReview added BreakingChangeReviewRequired <valid label in PR review process>add this label when breaking change review is required CI-FixRequiredOnFailure labels Dec 26, 2022
    @openapi-workflow-bot
    Copy link

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

    @openapi-workflow-bot
    Copy link

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

    @JeffreyRichter
    Copy link
    Member

    What is the justification for all the breaking changes shown here: https://github.com/Azure/azure-rest-api-specs/pull/21990/checks?check_run_id=10305263109

    @deyaaeldeen
    Copy link
    Member

    What is the justification for all the breaking changes shown here: https://github.com/Azure/azure-rest-api-specs/pull/21990/checks?check_run_id=10305263109

    @JeffreyRichter For context, the changes here are under the dev folder, which houses swaggers currently still in development and are not customer-facing yet.

    @kayousef
    Copy link
    Contributor

    What is the justification for all the breaking changes shown here: https://github.com/Azure/azure-rest-api-specs/pull/21990/checks?check_run_id=10305263109

    @JeffreyRichter As Deyaa mentioned, this is only in dev folder which is a WIP API version that is not released yet. Originally, we were planning for release in May 2023, but then decided to make it in April.
    Can you help merge this?

    @JeffreyRichter
    Copy link
    Member

    I don't understand. There was a path that shipped in the 2022-05-01 version and this new version is removing that path which is a breaking change. You say this new version is a WIP and not shipping yet - fair enough. But, when it does ship, it will then break customers. So, are we glad we caught this now so we do not break customers later? I'm afraid that if the Breaking Change Board approves this now, then it will eventually ship & break customers and we dont' want to do this.

    @kayousef
    Copy link
    Contributor

    kayousef commented Dec 30, 2022

    I don't understand. There was a path that shipped in the 2022-05-01 version and this new version is removing that path which is a breaking change. You say this new version is a WIP and not shipping yet - fair enough. But, when it does ship, it will then break customers. So, are we glad we caught this now so we do not break customers later? I'm afraid that if the Breaking Change Board approves this now, then it will eventually ship & break customers and we dont' want to do this.

    @JeffreyRichter can you clarify how does this change remove 2022-05-01?
    This PR changes 2023-05-01 to 2023-04-01 (note the year 2023 not 2022)

    @johanste
    Copy link
    Member

    If I understand correctly, the change is to move the API version for a not-yet-shipped API version to an earlier value. Technically, we don't have to make the change. The only reason is to move the API version closer to the expected ship date.

    Our breaking change tools do get royally confused by this, though :).

    @JeffreyRichter
    Copy link
    Member

    When I look at https://github.com/Azure/azure-rest-api-specs/pull/21990/checks?check_run_id=10305263109, see a table at the top with "new version" and "base version". In the table below, I see a "1005 - Removed Path" row saying that the path at https://github.com/Azure/azure-rest-api-specs/blob/main/specification/cognitiveservices/data-plane/Language/stable/2021-10-01/questionanswering-authoring.json#L30:5 no longer exists in the new PR version. It is a breaking change to customer when a path is removed.

    @kayousef
    Copy link
    Contributor

    kayousef commented Jan 1, 2023

    When I look at https://github.com/Azure/azure-rest-api-specs/pull/21990/checks?check_run_id=10305263109, see a table at the top with "new version" and "base version". In the table below, I see a "1005 - Removed Path" row saying that the path at https://github.com/Azure/azure-rest-api-specs/blob/main/specification/cognitiveservices/data-plane/Language/stable/2021-10-01/questionanswering-authoring.json#L30:5 no longer exists in the new PR version. It is a breaking change to customer when a path is removed.

    If you check the files changed, they are all in a "dev" folder that is used for under development API versions as per the policy we follow with the SDK crew.
    It seems the checks and validations get confused with this path, which is something we can discuss with @heaths to address with the tooling. But still, the change in this PR is not a breaking change as it was never released to customers.

    @JeffreyRichter
    Copy link
    Member

    Azure's definition of a breaking change is any change that requires customers to change their code. While this version has not yet been released to customers, it will then break them when it is ultimately released and therefore should not ever be released. Your team should be glad that these breaks are being detected early so that your customers will not ultimately be upset with the breaks damaging the reputation of your service and azure overall.

    @heaths
    Copy link
    Member

    heaths commented Jan 4, 2023

    What matters is that there are no breaking changes between GA versions under the specification/ folder. So if you're removing a path - even in the dev/ folder (same would be for a topic branch for specification/) - that was in GA and no longer is, that's a breaking change.

    @johanste johanste added the Approved-BreakingChange DO NOT USE! OBSOLETE label. See https://github.com/Azure/azure-sdk-tools/issues/6374 label Jan 17, 2023
    Copy link
    Member

    @jhendrixMSFT jhendrixMSFT left a comment

    Choose a reason for hiding this comment

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

    Please fix the model validation failure as it's blocking the merge.

    @jhendrixMSFT jhendrixMSFT merged commit deba715 into main Jan 19, 2023
    @jhendrixMSFT jhendrixMSFT deleted the kayousef/language/2023-04-01 branch January 19, 2023 16:38
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    Approved-BreakingChange DO NOT USE! OBSOLETE label. See https://github.com/Azure/azure-sdk-tools/issues/6374 BreakingChangeReviewRequired <valid label in PR review process>add this label when breaking change review is required Cognitive Services
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    8 participants