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

[Azure Stack] Updates to Azs.Fabric.Admin spec for autorest config #9030

Merged
merged 6 commits into from
Sep 4, 2020

Conversation

viananth
Copy link
Member

commit 6ecda64
Author: Yuxing Zhou zyx.pulsars@gmail.com
Date: Sat Jan 4 02:25:35 2020 +0800

[Azure Stack] Update fabric storage admin specs for new generation with autorest-beta (#8038)
commit 9e551f0eab4057d4c2f54c333c7aa2a1a564c125                                                                         Author: bganapa <bganapa@microsoft.com>                                                     Date:   Tue Nov 12 11:44:22 2019 -0800                                         Reset to Stackadmin2 (#7766)

Latest improvements:

MSFT employees can try out our new experience at OpenAPI Hub - one location for using our validation tools and finding your workflow.

Contribution checklist:

  • I have reviewed the documentation for the workflow.
  • Validation tools were run on swagger spec(s) and have all been fixed in this PR.
  • The OpenAPI Hub was used for checking validation status and next steps.

ARM API Review Checklist

  • Service team MUST add the "WaitForARMFeedback" label if the management plane API changes fall into one of the below categories.
  • adding/removing APIs.
  • adding/removing properties.
  • adding/removing API-version.
  • adding a new service in Azure.

Failure to comply may result in delays for manifest application. Note this does not apply to data plane APIs.

  • If you are blocked on ARM review and want to get the PR merged urgently, please get the ARM oncall for reviews (RP Manifest Approvers team under Azure Resource Manager service) from IcM and reach out to them.
    Please follow the link to find more details on API review process.

commit 6ecda64
Author: Yuxing Zhou <zyx.pulsars@gmail.com>
Date:   Sat Jan 4 02:25:35 2020 +0800

    [Azure Stack] Update fabric storage admin specs for new generation with autorest-beta (Azure#8038)
    commit 9e551f0                                                                         Author: bganapa <bganapa@microsoft.com>                                                     Date:   Tue Nov 12 11:44:22 2019 -0800                                         Reset to Stackadmin2 (Azure#7766)
@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Apr 13, 2020

azure-sdk-for-java - Release

No readme.md specification configuration files were found that are associated with the files modified in this pull request, or swagger_to_sdk section in readme.md is not configured

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Apr 13, 2020

azure-sdk-for-js - Release

No readme.md specification configuration files were found that are associated with the files modified in this pull request, or swagger_to_sdk section in readme.md is not configured

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Apr 13, 2020

azure-cli-extensions

No readme.md specification configuration files were found that are associated with the files modified in this pull request, or swagger_to_sdk section in readme.md is not configured

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Apr 13, 2020

azure-sdk-for-go - Release

No readme.md specification configuration files were found that are associated with the files modified in this pull request, or swagger_to_sdk section in readme.md is not configured

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Apr 13, 2020

azure-sdk-for-net - Release

No readme.md specification configuration files were found that are associated with the files modified in this pull request, or swagger_to_sdk section in readme.md is not configured

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Apr 13, 2020

azure-sdk-for-python - Release

No readme.md specification configuration files were found that are associated with the files modified in this pull request, or swagger_to_sdk section in readme.md is not configured

@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

@viananth
Copy link
Member Author

Can one of the admins verify this patch?

@zikalino Could you please approve this PR if it looks good to you? Thanks!

@viananth
Copy link
Member Author

@zikalino Could you please merge this PR? @bganapa as FYI

"$ref": "../2016-05-01/Fabric.json#/parameters/TopParameter"
},
{
"$ref": "../2016-05-01/Fabric.json#/parameters/SkipParameter"

Choose a reason for hiding this comment

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

referring files from different api version may be a bit dangerous and have some unexpected consequences.
i think it would be better to move these common definitions to a separate common.json file.
I see that it was done before, but maybe it's a good time to fix it?

Copy link
Member

Choose a reason for hiding this comment

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

agree, @viananth lets make a copy

Copy link

@zikalino zikalino left a comment

Choose a reason for hiding this comment

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

everything else looks fine for me, pls move params to common

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented May 26, 2020

Azure CLI Extension Generation - Release

No readme.md specification configuration files were found that are associated with the files modified in this pull request, or swagger_to_sdk section in readme.md is not configured

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented May 26, 2020

azure-sdk-for-trenton

No readme.md specification configuration files were found that are associated with the files modified in this pull request, or swagger_to_sdk section in readme.md is not configured

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@viananth
Copy link
Member Author

everything else looks fine for me, pls move params to common

@zikalino I have made the changes requested. Please take a look and let me know if it looks good. Thanks!

@mmyyrroonn
Copy link
Contributor

/azp run

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Jul 29, 2020

Trenton Generation - Release

No readme.md specification configuration files were found that are associated with the files modified in this pull request, or swagger_to_sdk section in readme.md is not configured

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Jul 29, 2020

azure-sdk-for-python-track2 - Release

No readme.md specification configuration files were found that are associated with the files modified in this pull request, or swagger_to_sdk section in readme.md is not configured

@mmyyrroonn
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@zhenglaizhang
Copy link
Contributor

/openapi pipeline

@openapi-sdkautomation
Copy link

/openapi pipeline

Unrecognized OpenAPI bot command (pipeline).
Try sdkautomation instead.

@openapi-pipeline-app
Copy link

openapi-pipeline-app bot commented Jul 29, 2020

[Staging] Swagger Validation Report

️✔️BreakingChange [Detail]
 There are no breaking changes. 
️✔️LintDiff [Detail]
 Validation passes for LintDiff. 
️✔️Avocado [Detail]
 Validation passes for Avocado. 
️✔️ModelValidation [Detail]
 Validation passes for ModelValidation. 
️✔️SemanticValidation [Detail]
 Validation passes for SemanticValidation. 
Posted by Swagger Pipeline | How to fix these errors?

@mmyyrroonn
Copy link
Contributor

@viananth Hi. @zikalino has joined a new team and cannot review this PR. I will help you merge this PR. According to our guideline, since you removed some paths, we need breaking change review. Could you share more context about why you removing these paths? You can check our review process and send a review request through this link

@viananth
Copy link
Member Author

@viananth Hi. @zikalino has joined a new team and cannot review this PR. I will help you merge this PR. According to our guideline, since you removed some paths, we need breaking change review. Could you share more context about why you removing these paths? You can check our review process and send a review request through this link

@bganapa @rakku-ms Could you pls respond to the above question as you have more context?

@bganapa
Copy link
Member

bganapa commented Jul 29, 2020

@viananth I think you are inadvertently removing the nascluster related files. Could you please add it back?

@viananth
Copy link
Member Author

Yes, looks like this commit was not backported in the PR: 3869a21#diff-68f3346e8107cf1e1ec3a5b93d33ffc3
I will rebase this branch.

@mmyyrroonn
Copy link
Contributor

@viananth These APIs are still deleted. If that's not on purpose, we can bring them back and we can go on the review process.

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@viananth
Copy link
Member Author

viananth commented Aug 3, 2020

@myronfanqiu I have ported the deleted files back. Apologies for the issue. Please merge the PR if it looks good to you. Thanks!

@@ -147,54 +146,3 @@ csharp:
output-folder: $(csharp-sdks-folder)/Generated
clear-output-folder: true
```

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi, why do we remove this part? Should we bring it back too?

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 part is added in readme.azsautogen.md for autorest generation config.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this one is used for autorest v3?

@mmyyrroonn
Copy link
Contributor

@zikalino Could you take a look at this PR and approve it?

Copy link
Contributor

@mmyyrroonn mmyyrroonn left a comment

Choose a reason for hiding this comment

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

I think we should revert the deletion in readme.md file.

@mmyyrroonn
Copy link
Contributor

@viananth Hi. Could you take a look at my comment and revert the change?

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@viananth
Copy link
Member Author

@viananth Hi. Could you take a look at my comment and revert the change?

@myronfanqiu I have reverted readme.md changes. Please merge the PR. Thanks!

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Aug 31, 2020

azure-resource-manager-schemas - Release

No readme.md specification configuration files were found that are associated with the files modified in this pull request, or swagger_to_sdk section in readme.md is not configured

@bganapa
Copy link
Member

bganapa commented Aug 31, 2020

@myronfanqiu please merge it

@mmyyrroonn
Copy link
Contributor

@akning-ms Hi. Could you help us merge this PR since @zikalino has moved to other team.

@akning-ms akning-ms merged commit 25acd0f into Azure:master Sep 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants