[Hub Generated] Review request for Microsoft.AlertsManagement to add version preview/2019-05-05-preview #5750

roramc · 2019-04-24T04:15:22Z

If you are a MSFT employee you can view your work branch via this link.

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.

…w/2018-11-02-privatepreview to version 2019-05-05-preview

AutorestCI · 2019-04-24T04:15:48Z

Automation for azure-sdk-for-js

Nothing to generate for azure-sdk-for-js

AutorestCI · 2019-04-24T04:15:55Z

Automation for azure-sdk-for-python

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

AutorestCI · 2019-04-24T04:16:04Z

Automation for azure-sdk-for-ruby

Nothing to generate for azure-sdk-for-ruby

AutorestCI · 2019-04-24T04:16:28Z

Automation for azure-sdk-for-java

Nothing to generate for azure-sdk-for-java

AutorestCI · 2019-04-24T04:17:46Z

Automation for azure-sdk-for-go

Nothing to generate for azure-sdk-for-go

azuresdkci · 2019-04-24T04:31:20Z

Can one of the admins verify this patch?

changed filters, introduced action rule type and configuration

added more examples and corrected some errors

veronicagg

@roramc there looks to be a syntax error in the swagger file, could you check https://dev.azure.com/azure-sdk/public/_build/results?buildId=21433&view=logs&jobId=5ea8264b-28a6-50c8-6f1c-6d360776db7e&taskId=cb0be435-f7f1-5f7a-37e4-9cbab5376524&lineStart=19&lineEnd=20&colStart=1&colEnd=1 ?

veronicagg · 2019-04-25T01:48:07Z

Tagging @KrisBash who was looking at the original PR from ARM side.

fixing swagger synatax error

fixing patch operation

fixing few model validation issues

corrected incorrect variables in examples

changing parameter in examples

roramc · 2019-04-25T08:29:45Z

@roramc there looks to be a syntax error in the swagger file, could you check https://dev.azure.com/azure-sdk/public/_build/results?buildId=21433&view=logs&jobId=5ea8264b-28a6-50c8-6f1c-6d360776db7e&taskId=cb0be435-f7f1-5f7a-37e4-9cbab5376524&lineStart=19&lineEnd=20&colStart=1&colEnd=1 ?

fixed the syntax issue/model validation, etc.

veronicagg · 2019-04-26T00:49:14Z

Please review errors at https://portal.azure-devex-tools.com/app/branch/roramc/azure-rest-api-specs/dev-alertsmanagement-Microsoft.AlertsManagement-2019-05-05-preview...master/linter particularly the ones under "IsNew" column = true.
Property names like https://github.com/roramc/azure-rest-api-specs/blob/48467c63614bb04de4535ee9da6db3a232089714/specification/alertsmanagement/resource-manager/Microsoft.AlertsManagement/preview/2019-05-05-preview/AlertsManagement.json#L1687 should be camel cased without spaces.
Please check error generating Python SDK https://dev.azure.com/azure-sdk/public/_build/results?buildId=22034&view=logs&jobId=957f396e-b28a-53c4-b5bb-b8c6d959bd2d&taskId=142feabb-6d02-5968-caa1-f5b3c04c5cae&lineStart=136&lineEnd=137&colStart=1&colEnd=1

veronicagg

minor: we generally use 2 spaces for indentation. See pointer to other CI errors above.

fixing resource group parameter error

roramc · 2019-04-26T11:43:38Z

Please review errors at https://portal.azure-devex-tools.com/app/branch/roramc/azure-rest-api-specs/dev-alertsmanagement-Microsoft.AlertsManagement-2019-05-05-preview...master/linter particularly the ones under "IsNew" column = true.
Property names like https://github.com/roramc/azure-rest-api-specs/blob/48467c63614bb04de4535ee9da6db3a232089714/specification/alertsmanagement/resource-manager/Microsoft.AlertsManagement/preview/2019-05-05-preview/AlertsManagement.json#L1687 should be camel cased without spaces.
Please check error generating Python SDK https://dev.azure.com/azure-sdk/public/_build/results?buildId=22034&view=logs&jobId=957f396e-b28a-53c4-b5bb-b8c6d959bd2d&taskId=142feabb-6d02-5968-caa1-f5b3c04c5cae&lineStart=136&lineEnd=137&colStart=1&colEnd=1

I have fixed most of the lint errors, the camel case error are for the contract of an existing resource type and existing version. So I can't change those properties. Also, we are just adding one new resource type for this version, should I remove the other existing resource types from the swagger and examples?

veronicagg · 2019-04-26T18:08:48Z

@roramc please follow up with the ARM team on signing off for this PR, since they haven't replied here yet. Regarding your question on removing existing resource types from swagger and examples, if they are part of the service you're specifying they should remain there, why would you remove them? thanks!

roramc · 2019-04-29T13:36:05Z

@roramc please follow up with the ARM team on signing off for this PR, since they haven't replied here yet. Regarding your question on removing existing resource types from swagger and examples, if they are part of the service you're specifying they should remain there, why would you remove them? thanks!

The other resource types are already in GA. So we were thinking of just having the new resource type with the preview version. Not sure what is the convention around this.

KrisBash

LGTM

veronicagg · 2019-05-01T01:16:54Z

@roramc have you reviewed linter warnings and errors?
For example:

https://dev.azure.com/azure-sdk/public/_build/results?buildId=23706&view=logs&jobId=688669d0-441c-57c3-cf6d-f89a22ccfa5d&taskId=923b5a90-e462-5994-c166-86509c6bf5f2&lineStart=902&lineEnd=903&colStart=1&colEnd=1 should the operation be marked as x-ms-pageable?
https://dev.azure.com/azure-sdk/public/_build/results?buildId=23706&view=logs&jobId=688669d0-441c-57c3-cf6d-f89a22ccfa5d&taskId=923b5a90-e462-5994-c166-86509c6bf5f2&lineStart=906&lineEnd=907&colStart=1&colEnd=1 should the operation name be updated?
https://dev.azure.com/azure-sdk/public/_build/results?buildId=23706&view=logs&jobId=688669d0-441c-57c3-cf6d-f89a22ccfa5d&taskId=923b5a90-e462-5994-c166-86509c6bf5f2&lineStart=918&lineEnd=919&colStart=1&colEnd=1 should this name be updated?
https://dev.azure.com/azure-sdk/public/_build/results?buildId=23706&view=logs&jobId=688669d0-441c-57c3-cf6d-f89a22ccfa5d&taskId=923b5a90-e462-5994-c166-86509c6bf5f2&lineStart=954&lineEnd=955&colStart=1&colEnd=1 did you consider flattening?

fixed few linter warnings

roramc · 2019-05-01T15:33:07Z

@roramc have you reviewed linter warnings and errors?
For example:

https://dev.azure.com/azure-sdk/public/_build/results?buildId=23706&view=logs&jobId=688669d0-441c-57c3-cf6d-f89a22ccfa5d&taskId=923b5a90-e462-5994-c166-86509c6bf5f2&lineStart=902&lineEnd=903&colStart=1&colEnd=1 should the operation be marked as x-ms-pageable?

https://dev.azure.com/azure-sdk/public/_build/results?buildId=23706&view=logs&jobId=688669d0-441c-57c3-cf6d-f89a22ccfa5d&taskId=923b5a90-e462-5994-c166-86509c6bf5f2&lineStart=906&lineEnd=907&colStart=1&colEnd=1 should the operation name be updated?

https://dev.azure.com/azure-sdk/public/_build/results?buildId=23706&view=logs&jobId=688669d0-441c-57c3-cf6d-f89a22ccfa5d&taskId=923b5a90-e462-5994-c166-86509c6bf5f2&lineStart=918&lineEnd=919&colStart=1&colEnd=1 should this name be updated?

https://dev.azure.com/azure-sdk/public/_build/results?buildId=23706&view=logs&jobId=688669d0-441c-57c3-cf6d-f89a22ccfa5d&taskId=923b5a90-e462-5994-c166-86509c6bf5f2&lineStart=954&lineEnd=955&colStart=1&colEnd=1 did you consider flattening?

fixed all the above and few more as well for the new resource type "actionRules". The remaining are for the old resource types.

veronicagg

left one more question inline, rest looks good. thanks!

veronicagg · 2019-05-01T21:49:00Z

...resource-manager/Microsoft.AlertsManagement/preview/2019-05-05-preview/AlertsManagement.json

            "schema": {
-              "$ref": "#/definitions/smartGroupsList",
-              "description": "List of smart groups in value property."
+              "$ref": "#/definitions/smartGroupsList"


names for model definitions should be capitalized for consistency, I noticed you have some starting with lower case and some upper case in the "definitions" section.

Changed models to upper case

…-05-preview' of https://github.com/roramc/azure-rest-api-specs into dev-alertsmanagement-Microsoft.AlertsManagement-2019-05-05-preview

fixing duplicate issues due to merge conflicts

veronicagg

@roramc Thanks for addressing the comments!

roramc added 3 commits April 24, 2019 09:42

Adds base for updating Microsoft.AlertsManagement from version previe…

bd28072

…w/2018-11-02-privatepreview to version 2019-05-05-preview

Updates readme

ba3d33a

Updates API version in new specs and examples

48467c6

AutorestCI mentioned this pull request Apr 24, 2019

[AutoPR alertsmanagement/resource-manager] [Hub Generated] Review request for Microsoft.AlertsManagement to add version preview/2019-05-05-preview Azure/azure-sdk-for-go#4641

Closed

azuresdkci assigned sergey-shandar Apr 24, 2019

azuresdkci requested a review from sergey-shandar April 24, 2019 04:27

roramcmsft added 2 commits April 24, 2019 10:12

Update AlertsManagement.json

3442df6

changed filters, introduced action rule type and configuration

added more examples and corrected some errors

7e30b33

added more examples and corrected some errors

veronicagg mentioned this pull request Apr 25, 2019

Adding action rule preview version #5712

Closed

5 tasks

veronicagg assigned veronicagg and unassigned sergey-shandar Apr 25, 2019

veronicagg suggested changes Apr 25, 2019

View reviewed changes

veronicagg added the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Apr 25, 2019

roramcmsft added 6 commits April 25, 2019 10:16

fixing swagger synatax error

8c8625f

fixing swagger synatax error

fixing some lint and syntax issues

28b9906

fixing patch operation

53a2e4a

fixing patch operation

fixing few model validation issues

29fb168

fixing few model validation issues

corrected incorrect variables in examples

605d59b

corrected incorrect variables in examples

changing parameter in examples

d4eeb01

changing parameter in examples

veronicagg suggested changes Apr 26, 2019

View reviewed changes

fixing resource group parameter error

601789e

fixing resource group parameter error

AutorestCI mentioned this pull request Apr 26, 2019

[AutoPR alertsmanagement/resource-manager] [Hub Generated] Review request for Microsoft.AlertsManagement to add version preview/2019-05-05-preview Azure/azure-sdk-for-python#4966

Merged

roramc closed this Apr 29, 2019

roramc reopened this Apr 30, 2019

KrisBash approved these changes Apr 30, 2019

View reviewed changes

KrisBash 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 Apr 30, 2019

roramcmsft and others added 2 commits May 1, 2019 20:43

fixed linter warnings

3cf6bb7

fixed few linter warnings

ce2b27a

fixed few linter warnings

veronicagg suggested changes May 1, 2019

View reviewed changes

roramcmsft added 3 commits May 2, 2019 10:39

Changed models to upper case

0e1f322

Changed models to upper case

Merge branch 'dev-alertsmanagement-Microsoft.AlertsManagement-2019-05…

7704911

…-05-preview' of https://github.com/roramc/azure-rest-api-specs into dev-alertsmanagement-Microsoft.AlertsManagement-2019-05-05-preview

fixing duplicate issues due to merge conflicts

fbda595

fixing duplicate issues due to merge conflicts

veronicagg approved these changes May 2, 2019

View reviewed changes

veronicagg merged commit 56de8e7 into Azure:master May 2, 2019

[Hub Generated] Review request for Microsoft.AlertsManagement to add version preview/2019-05-05-preview #5750

[Hub Generated] Review request for Microsoft.AlertsManagement to add version preview/2019-05-05-preview #5750

Uh oh!

Conversation

roramc commented Apr 24, 2019

Contribution checklist:

Uh oh!

AutorestCI commented Apr 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Automation for azure-sdk-for-js

Uh oh!

AutorestCI commented Apr 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Automation for azure-sdk-for-python

Uh oh!

AutorestCI commented Apr 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Automation for azure-sdk-for-ruby

Uh oh!

AutorestCI commented Apr 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Automation for azure-sdk-for-java

Uh oh!

AutorestCI commented Apr 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Automation for azure-sdk-for-go

Uh oh!

azuresdkci commented Apr 24, 2019

Uh oh!

veronicagg left a comment

Choose a reason for hiding this comment

Uh oh!

veronicagg commented Apr 25, 2019

Uh oh!

roramc commented Apr 25, 2019

Uh oh!

veronicagg commented Apr 26, 2019

Uh oh!

veronicagg left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

roramc commented Apr 26, 2019

Uh oh!

veronicagg commented Apr 26, 2019

Uh oh!

roramc commented Apr 29, 2019

Uh oh!

KrisBash left a comment

Choose a reason for hiding this comment

Uh oh!

veronicagg commented May 1, 2019

Uh oh!

roramc commented May 1, 2019

Uh oh!

veronicagg left a comment

Choose a reason for hiding this comment

Uh oh!

veronicagg May 1, 2019

Choose a reason for hiding this comment

Uh oh!

roramc May 2, 2019

Choose a reason for hiding this comment

Uh oh!

veronicagg left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

AutorestCI commented Apr 24, 2019 •

edited

Loading

AutorestCI commented Apr 24, 2019 •

edited

Loading

AutorestCI commented Apr 24, 2019 •

edited

Loading

AutorestCI commented Apr 24, 2019 •

edited

Loading

AutorestCI commented Apr 24, 2019 •

edited

Loading

veronicagg left a comment •

edited

Loading