Skip to content

Conversation

@amolagar5
Copy link
Contributor

@amolagar5 amolagar5 commented Oct 28, 2019

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.

@amolagar5 amolagar5 requested a review from jaredmoo as a code owner October 28, 2019 21:21
@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Oct 28, 2019

In Testing, Please Ignore

[Logs] (Generated from 447f626, Iteration 3)

In-Progress .NET: test-repo-billy/azure-sdk-for-net [Logs]
  • Package generation in progress.
In-Progress Python: test-repo-billy/azure-sdk-for-python [Logs]
  • Package generation in progress.
In-Progress Java: test-repo-billy/azure-sdk-for-java [Logs]
  • Package generation in progress.
In-Progress Go: test-repo-billy/azure-sdk-for-go [Logs]
  • Package generation in progress.
In-Progress JavaScript: test-repo-billy/azure-sdk-for-js [Logs]
  • Package generation in progress.
Pending Ruby: test-repo-billy/azure-sdk-for-ruby
  • Package generation pending.

@msftclas
Copy link

msftclas commented Oct 28, 2019

CLA assistant check
All CLA requirements met.

@AutorestCI
Copy link

AutorestCI commented Oct 28, 2019

Automation for azure-sdk-for-go

A PR has been created for you based on this PR content.

Once this PR will be merged, content will be added to your service PR:
Azure/azure-sdk-for-go#6438

@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

@lirenhe
Copy link
Member

lirenhe commented Oct 29, 2019

@amolagar5 , could you take a look at the tool check error? It seems "$ref": "#/definitions/ProxyResource" is not defined.

@lirenhe lirenhe added WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required Changes Required labels Oct 29, 2019
@lirenhe
Copy link
Member

lirenhe commented Oct 31, 2019

@amolagar5 , I still see a lot of check error, fix take a look at them and do the proper fix. And let me know if you have any questions.

@amolagar5
Copy link
Contributor Author

Thanks @lirenhe I will take a look and update the request.

@AutorestCI
Copy link

AutorestCI commented Nov 1, 2019

Automation for azure-sdk-for-python

The initial PR has been merged into your service PR:
Azure/azure-sdk-for-python#8720

@amolagar5
Copy link
Contributor Author

@lirenhe , can you please help me figure out why checks are failing. I keep getting something like this.
INFO:swaggertosdk.autorest_tools: Error: '$.paths["/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Sql/servers/{serverName}/administrators/{administratorName}"].put.tags' has incompatible values (---

INFO:swaggertosdk.autorest_tools:- ServerAdministrators

INFO:swaggertosdk.autorest_tools:, ---

INFO:swaggertosdk.autorest_tools:- Administrators

INFO:swaggertosdk.autorest_tools:).

@lirenhe
Copy link
Member

lirenhe commented Nov 4, 2019

@amolagar, could you first fix the problem in Avocado and ModelValidation check? This may help to solve the problem for autorest.

@amolagar5
Copy link
Contributor Author

@lirenhe thanks Renhe for taking a look, Looks like swaggers for 2014-04-01 and 2018-06-01-preview have to be exactly same for create or update. Any idea why that is needed. ?

@lirenhe
Copy link
Member

lirenhe commented Nov 5, 2019

@lirenhe thanks Renhe for taking a look, Looks like swaggers for 2014-04-01 and 2018-06-01-preview have to be exactly same for create or update. Any idea why that is needed. ?

Not quite sure about this. What item do you think is 'not needed'?

@lirenhe
Copy link
Member

lirenhe commented Nov 6, 2019

@amolagar, there are a couple of failures in prettier check, could you also fix the code style issue?

@amolagar5
Copy link
Contributor Author

@lirenhe how do I fix these prettier check failures, it only says check fails. Does not say why and how to fix them. Can you please share more details on what to fix and how to run tool locally or on git to find more information.

@PhoenixHe-NV
Copy link

@qiaozha @amolagar5 Fixed.

"$ref": "#/parameters/ServerNameParameter"
},
{
"name": "administratorName",
Copy link
Contributor

Choose a reason for hiding this comment

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

}
}
},
"AdministratorListResult": {
Copy link
Contributor

Choose a reason for hiding this comment

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

@amolagar5
Copy link
Contributor Author

@lirenhe whom should I ask for merge now. Is it ARM team. ?

@@ -1,314 +0,0 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you gone through the deprecation process for these APIs?

https://armwiki.azurewebsites.net/api_contracts/APIDeprecationPolicy.html

"swagger": "2.0",
"info": {
"version": "2018-06-01-preview",
"title": "SqlManagementClient",
Copy link
Contributor

Choose a reason for hiding this comment

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

Title should be human readable

"type": "string",
"x-ms-parameter-location": "method"
},
"SqlVirtualMachineContainerNameParameter": {
Copy link
Contributor

Choose a reason for hiding this comment

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

unused

],
"x-ms-parameter-location": "method"
},
"SqlVirtualMachineInstanceNameParameter": {
Copy link
Contributor

Choose a reason for hiding this comment

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

unused

"type": "string",
"x-ms-parameter-location": "method"
},
"VirtualClusterNameParameter": {
Copy link
Contributor

Choose a reason for hiding this comment

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

unused

"type": "string",
"x-ms-parameter-location": "method"
},
"BlobAuditingPolicyNameParameter": {
Copy link
Contributor

Choose a reason for hiding this comment

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

unused

"type": "string",
"x-ms-parameter-location": "method"
},
"ManagedInstanceNameParameter": {
Copy link
Contributor

Choose a reason for hiding this comment

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

unused

"type": "string",
"x-ms-parameter-location": "method"
},
"DatabaseNameParameter": {
Copy link
Contributor

Choose a reason for hiding this comment

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

unused

- Microsoft.Sql/stable/2014-04-01/importExport.json
- Microsoft.Sql/stable/2014-04-01/metrics.json
- Microsoft.Sql/stable/2014-04-01/replicationLinks.json
- Microsoft.Sql/stable/2014-04-01/serverAzureADAdministrators.json
Copy link
Contributor

Choose a reason for hiding this comment

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

Should not remove

- Microsoft.Sql/stable/2014-04-01/importExport.json
- Microsoft.Sql/stable/2014-04-01/metrics.json
- Microsoft.Sql/stable/2014-04-01/replicationLinks.json
- Microsoft.Sql/stable/2014-04-01/serverAzureADAdministrators.json
Copy link
Contributor

Choose a reason for hiding this comment

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

Should not remove

@amolagar5
Copy link
Contributor Author

@ryansbenson added old files back, for title. - @jaredmoo said its generic one for all SQL, unused params are generated by automation and not worth removing manually.

Copy link
Contributor

@ryansbenson ryansbenson left a comment

Choose a reason for hiding this comment

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

Approved from ARMs side

@ryansbenson ryansbenson added ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review and removed WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required labels Nov 22, 2019
@amolagar5
Copy link
Contributor Author

@lirenhe , can you please merge. thanks @ryansbenson for signing off.

@lirenhe
Copy link
Member

lirenhe commented Nov 27, 2019

@amolagar5, I am unable to merge it because of the conflicts. Please help to resolve the conflicts.

…aggerChanges

# Conflicts:
#	specification/sql/resource-manager/readme.md
@amolagar5
Copy link
Contributor Author

@lirenhe , Sure. Merge complete, pls try again.

@lirenhe lirenhe merged commit 1fa24f1 into Azure:master Nov 28, 2019
@amolagar
Copy link
Contributor

amolagar commented Nov 28, 2019 via email

TalluriAnusha pushed a commit to AsrOneSdk/azure-rest-api-specs that referenced this pull request Dec 11, 2019
* Add new API for Server AAD Admin

* Add new Serer AAD Admin API version

* Add new API version to Server AAD Admin API

* Add new API version to Server AAD Admin API

* New Server AAD Admin API

* fix examples

* fix AAD example errors

* fix AAD example errors

* fix AAD example errors 2

* fix AAD json

* fix tags in json

* update json

* update server admin jsons

* update server admin jsons

* updates

* fix 2014 AAD Admin json

* fix 2014 AAD Admin json

* fix AAD Admin json

* fix AAD Admin json

* fix AAD Admin json

* fix create example

* remove new example files.

* add back new example files.

* add back new example files.

* add back new example files.

* fixing example files

* fix tag in old Server AAD Admin json

* use common values

* remove extra value

* use common values

* use common values

* remove old api version docs

* adding new serverAzureADADministrator.json

* adding new serverAzureADADministrator.json

* Doing Prettier fixes

* Make administratorName an enum. as ActiveDirectory is constant.

* fixing syntax

* update example files with right casing for ActiveDirectory enum

* add backing 2014 version files and updating readme.md

* fix a few things.

* extra spaces.

* Add back json and update readme.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants