AdministratorName parameter should take value which is defined in enum #7377

SanjaMalesevic · 2019-10-01T18:06:26Z

No description provided.

openapi-sdkautomation · 2019-10-01T18:06:29Z

In Testing, Please Ignore

[Logs] (Generated from 7555722, Iteration 3)

.NET: test-repo-billy/azure-sdk-for-net [Logs] [Diff]

No packages generated.

Python: test-repo-billy/azure-sdk-for-python [Logs] [Diff]

azure-mgmt-sql [Logs] [Instructions] [Generation PR]
- azure-mgmt-sql-0.13.0.zip
- azure_mgmt_sql-0.13.0-py2.py3-none-any.whl

Java: test-repo-billy/azure-sdk-for-java [Logs] [Diff]

sql/resource-manager/v2014_04_01 [Logs] [Instructions]
sql/resource-manager/v2015_05_01_preview [Logs]
sql/resource-manager/v2017_03_01_preview [Logs]
sql/resource-manager/v2017_10_01_preview [Logs]
sql/resource-manager/v2018_06_01_preview [Logs]

Go: test-repo-billy/azure-sdk-for-go [Logs] [Diff]

preview/sql/mgmt/2015-05-01-preview [Logs] [Instructions] [Generation PR]
- preview.sql.mgmt.2015-05-01-preview.zip
preview/sql/mgmt/2017-03-01-preview [Logs] [Instructions] [Generation PR]
- preview.sql.mgmt.2017-03-01-preview.zip
preview/sql/mgmt/2017-10-01-preview [Logs] [Instructions] [Generation PR]
- preview.sql.mgmt.2017-10-01-preview.zip
preview/sql/mgmt/2018-06-01-preview [Logs] [Instructions] [Generation PR]
- preview.sql.mgmt.2018-06-01-preview.zip
sql/mgmt/2014-04-01 [Logs] [Instructions] [Generation PR]
- sql.mgmt.2014-04-01.zip

JavaScript: test-repo-billy/azure-sdk-for-js [Logs] [Diff]

@azure/arm-sql [Logs] [Instructions]
- azure-arm-sql-5.7.0.tgz

Ruby: test-repo-billy/azure-sdk-for-ruby [Logs] [Diff]

azure_sdk [Logs] [Instructions]
- azure_sdk-0.19.7.gem

azuresdkci · 2019-10-01T18:07:41Z

Can one of the admins verify this patch?

Merge changes from master

AutorestCI · 2019-10-01T18:08:43Z

Automation for azure-sdk-for-python

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-python#8425

AutorestCI · 2019-10-01T18:10:26Z

Automation for azure-sdk-for-go

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

tjprescott · 2019-10-23T17:21:28Z

...resource-manager/Microsoft.Sql/preview/2017-03-01-preview/managedInstanceAdministrators.json

-            "type": "string"
+            "type": "string",
+			"enum": [
+				"ActiveDirectory"


Does the change from string to enum result in a breaking change for some SDKs?

Currently, there is a difference in number of parameters for create/modify/delete methods in .NET and Python clients between MI and SQL DB. So, this change is made to aligned MI clients with SQL DB (they need to be auto-generated again based on updated json).
Jared has noticed in PR Azure/azure-cli#10446 that there is parameter AdministratorName in method create_or_update which always takes the same value and that this parameter doesn't exist in clients for sql db.
I am not sure, but I think that this is a breaking change.

Yes, it is a breaking change. However this is a new API and therefore right now is the time that break will have minimal impact. Also this change brings managed instance admin API into sync with server admin API.

SanjaMalesevic · 2019-11-04T17:12:30Z

Hi all, this is a gentle reminder to take look at this PR.
Thanks,
Sanja

tjprescott · 2019-11-04T17:48:03Z

@Azure/arm-api-review-board please take a look.

...resource-manager/Microsoft.Sql/preview/2017-03-01-preview/managedInstanceAdministrators.json

majastrz

I added a comment. Please take a look.

majastrz · 2019-11-04T19:50:23Z

Some of the CI/CD checks are failing on this PR also. Please take a look.

SanjaMalesevic · 2019-11-06T18:02:08Z

Some of the CI/CD checks are failing on this PR also. Please take a look.

I saw from the error log that ModelValidation failed when trying to validate "examples" and "x-ms-examples", but there is no specific error here. @majastrz could you please help me to figure out what is wrong here?

majastrz · 2019-11-06T21:36:50Z

If you keep clicking links on the Details next to the failing check, you will eventually get to the Model Validation errors.

I copied the following from https://dev.azure.com/azure-sdk/public/_build/results?buildId=181455&view=logs&j=8d2f9c49-cd83-5a9d-f3fe-2fd651afa742&t=43324f1f-6724-5d61-785f-f0ac9fd256a1:

 error : 
operationId: ManagedInstanceAdministrators_CreateOrUpdate
scenario: Update administrator of managed instance
source: response
responseCode: '200'
severity: 0
code: OBJECT_ADDITIONAL_PROPERTIES
details:
  code: OBJECT_ADDITIONAL_PROPERTIES
  params:
    - location
  message: 'Additional properties not allowed: location'
  path: ''
  title: '#/definitions/ManagedInstanceAdministrator'
  description: An Azure SQL managed instance administrator.
  position:
    line: 325
    column: 37
  url: >-
    specification/sql/resource-manager/Microsoft.Sql/preview/2017-03-01-preview/managedInstanceAdministrators.json
  directives: {}
  jsonPosition:
    line: 19
    column: 15
  jsonUrl: >-
    specification/sql/resource-manager/Microsoft.Sql/preview/2017-03-01-preview/examples/ManagedInstanceAdministratorUpdate.json
  jsonPath: ''

 error : 
operationId: ManagedInstanceAdministrators_CreateOrUpdate
scenario: Update administrator of managed instance
source: response
responseCode: '201'
severity: 0
code: OBJECT_ADDITIONAL_PROPERTIES
details:
  code: OBJECT_ADDITIONAL_PROPERTIES
  params:
    - location
  message: 'Additional properties not allowed: location'
  path: ''
  title: '#/definitions/ManagedInstanceAdministrator'
  description: An Azure SQL managed instance administrator.
  position:
    line: 325
    column: 37
  url: >-
    specification/sql/resource-manager/Microsoft.Sql/preview/2017-03-01-preview/managedInstanceAdministrators.json
  directives: {}
  jsonPosition:
    line: 33
    column: 15
  jsonUrl: >-
    specification/sql/resource-manager/Microsoft.Sql/preview/2017-03-01-preview/examples/ManagedInstanceAdministratorUpdate.json
  jsonPath: ''

##[error]Bash exited with code '1'.
Finishing: Model Validation

It appears that there's an issue with the examples referencing a property that is not declared in the schema.

majastrz

Signed off from ARM side.

tjprescott · 2019-11-06T23:11:37Z

/azurepipelines run

tjprescott · 2019-11-06T23:11:59Z

/azurepipelines run

tjprescott · 2019-11-06T23:21:48Z

Please check on the build failure: https://dev.azure.com/azure-sdk/public/_build/results?buildId=181455&view=logs&j=d2a54e27-891b-5b91-4b1b-0c6d3d820755&t=4db5f656-3b59-5661-54ad-d4645c211b0f

SanjaMalesevic · 2019-11-07T10:10:38Z

I've fixed the examples and all checks have passed. Thanks for help.

Azure#7377) * AdministratorName parameter should take constant value defined in enum * Code review fixes * Code review fixes * Fix tabs and spaces * Fix example with get * revert example with get * Fix examples

AdministratorName parameter should take constant value defined in enum

b0dae31

SanjaMalesevic requested a review from jaredmoo as a code owner October 1, 2019 18:06

azuresdkci assigned tjprescott Oct 1, 2019

azuresdkci requested a review from tjprescott October 1, 2019 18:07

Merge pull request #1 from Azure/master

e0829d2

Merge changes from master

AutorestCI mentioned this pull request Oct 1, 2019

[AutoPR sql/resource-manager] Change AdministratorName parameter, it should take constant value defined in enum Azure/azure-sdk-for-python#7534

Closed

Merge branch 'master' into changeJsonForADAdminoOnMI

7555722

AutorestCI mentioned this pull request Oct 1, 2019

[AutoPR sql/resource-manager] Change AdministratorName parameter, it should take constant value defined in enum Azure/azure-sdk-for-go#5929

Closed

openapi-sdkautomation bot mentioned this pull request Oct 1, 2019

[AutoPR sql/resource-manager/v2014_04_01] Change AdministratorName parameter, it should take constant value defined in enum test-repo-billy/azure-sdk-for-java#499

Closed

SanjaMalesevic changed the title ~~Change AdministratorName parameter, it should take constant value defined in enum~~ AdministratorName parameter should take value which is defined in enum Oct 2, 2019

jaredmoo approved these changes Oct 23, 2019

View reviewed changes

tjprescott added the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Oct 23, 2019

tjprescott reviewed Oct 23, 2019

View reviewed changes

majastrz reviewed Nov 4, 2019

View reviewed changes

...resource-manager/Microsoft.Sql/preview/2017-03-01-preview/managedInstanceAdministrators.json Outdated Show resolved Hide resolved

majastrz reviewed Nov 4, 2019

View reviewed changes

...resource-manager/Microsoft.Sql/preview/2017-03-01-preview/managedInstanceAdministrators.json Outdated Show resolved Hide resolved

majastrz added the ARMChangesRequested <valid label in PR review process>add this label when require changes after ARM review label Nov 4, 2019

majastrz requested changes Nov 4, 2019

View reviewed changes

SanjaMalesevic added 2 commits November 6, 2019 15:09

Code review fixes

32a1325

Code review fixes

f20be64

SanjaMalesevic added 2 commits November 6, 2019 18:03

Fix tabs and spaces

d8f256c

Fix example with get

31de475

revert example with get

57fa02d

majastrz approved these changes Nov 6, 2019

View reviewed changes

tjprescott approved these changes Nov 6, 2019

View reviewed changes

Fix examples

e765474

tjprescott merged commit 5e95a31 into Azure:master Nov 7, 2019

SanjaMalesevic mentioned this pull request Nov 8, 2019

Update operations for Azure AD administrator on MI. Azure/azure-sdk-for-net#8685

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AdministratorName parameter should take value which is defined in enum #7377

AdministratorName parameter should take value which is defined in enum #7377

SanjaMalesevic commented Oct 1, 2019

openapi-sdkautomation bot commented Oct 1, 2019 •

edited

Loading

azuresdkci commented Oct 1, 2019

AutorestCI commented Oct 1, 2019 •

edited

Loading

AutorestCI commented Oct 1, 2019 •

edited

Loading

tjprescott Oct 23, 2019

SanjaMalesevic Oct 24, 2019 •

edited

Loading

jaredmoo Oct 24, 2019

SanjaMalesevic commented Nov 4, 2019

tjprescott commented Nov 4, 2019

majastrz left a comment

majastrz commented Nov 4, 2019

SanjaMalesevic commented Nov 6, 2019

majastrz commented Nov 6, 2019 •

edited

Loading

majastrz left a comment

tjprescott commented Nov 6, 2019

tjprescott commented Nov 6, 2019

tjprescott commented Nov 6, 2019

SanjaMalesevic commented Nov 7, 2019

AdministratorName parameter should take value which is defined in enum #7377

AdministratorName parameter should take value which is defined in enum #7377

Conversation

SanjaMalesevic commented Oct 1, 2019

openapi-sdkautomation bot commented Oct 1, 2019 • edited Loading

In Testing, Please Ignore

[Logs] (Generated from 7555722, Iteration 3)

azuresdkci commented Oct 1, 2019

AutorestCI commented Oct 1, 2019 • edited Loading

Automation for azure-sdk-for-python

AutorestCI commented Oct 1, 2019 • edited Loading

Automation for azure-sdk-for-go

tjprescott Oct 23, 2019

Choose a reason for hiding this comment

SanjaMalesevic Oct 24, 2019 • edited Loading

Choose a reason for hiding this comment

jaredmoo Oct 24, 2019

Choose a reason for hiding this comment

SanjaMalesevic commented Nov 4, 2019

tjprescott commented Nov 4, 2019

majastrz left a comment

Choose a reason for hiding this comment

majastrz commented Nov 4, 2019

SanjaMalesevic commented Nov 6, 2019

majastrz commented Nov 6, 2019 • edited Loading

majastrz left a comment

Choose a reason for hiding this comment

tjprescott commented Nov 6, 2019

tjprescott commented Nov 6, 2019

tjprescott commented Nov 6, 2019

SanjaMalesevic commented Nov 7, 2019

openapi-sdkautomation bot commented Oct 1, 2019 •

edited

Loading

AutorestCI commented Oct 1, 2019 •

edited

Loading

AutorestCI commented Oct 1, 2019 •

edited

Loading

SanjaMalesevic Oct 24, 2019 •

edited

Loading

majastrz commented Nov 6, 2019 •

edited

Loading