[Azure Analysis Services] Enable associate/dissociate gateway with AS. #6043

taiwu · 2018-04-25T00:25:12Z

Description

Checklist

I have read the Submitting Changes section of CONTRIBUTING.md
The title of the PR is clear and informative
The appropriate change log has been updated
The PR does not introduce breaking changes
If applicable, the changes made in the PR have proper test coverage
For public API changes to cmdlets:
- the changes have gone through a cmdlet design review
- the cmdlet markdown files were generated using the platyPS module

markcowl · 2018-04-25T02:25:01Z

...ager/AnalysisServices/Commands.AnalysisServices/Commands/NewAzureRmAnalysisServicesServer.cs

@@ -82,6 +82,18 @@ public class NewAnalysisServicesServer : AnalysisServicesCmdletBase
            HelpMessage = "Firewall configuration")]
        public PsAzureAnalysisServicesFirewallConfig FirewallConfig { get; set; }

+        [Parameter(ValueFromPipelineByPropertyName = true, Mandatory = false,
+            HelpMessage = "Gateway resource name")]


You should allow them to specify the gateway by resource ID instead, they are much more likely to have this value

We construct the resource ID internally, but sure.

markcowl · 2018-04-25T02:25:19Z

...ager/AnalysisServices/Commands.AnalysisServices/Commands/SetAzureRmAnalysisServicesServer.cs

@@ -88,6 +88,22 @@ public int ReadonlyReplicaCount
            HelpMessage = "Firewall configuration")]
        public PsAzureAnalysisServicesFirewallConfig FirewallConfig { get; set; }

+        [Parameter(ValueFromPipelineByPropertyName = true, Mandatory = false,


same comment

@markcowl updated, please review again.

These should be in separate parameter sets.

@markcowl updated

taiwu · 2018-04-26T18:36:49Z

ping @markcowl

taiwu · 2018-05-01T02:57:42Z

ping @markcowl and @maddieclayton

markcowl · 2018-05-01T23:16:55Z

@taiwu You need to have a cmdlet review for any public api changes: https://github.com/azure/azure-powershell-cmdlet-review-pr/issues

markcowl · 2018-05-01T23:18:45Z

...ager/AnalysisServices/Commands.AnalysisServices/Commands/SetAzureRmAnalysisServicesServer.cs

@@ -88,6 +88,22 @@ public int ReadonlyReplicaCount
            HelpMessage = "Firewall configuration")]
        public PsAzureAnalysisServicesFirewallConfig FirewallConfig { get; set; }

+        [Parameter(ValueFromPipelineByPropertyName = true, Mandatory = false,


These should be in separate parameter sets.

taiwu · 2018-05-01T23:26:13Z

cmdlet review already added last week. https://github.com/Azure/azure-powershell-pr/wiki/Cmdlet-Reviews

taiwu · 2018-05-04T20:42:13Z

Updated the parameter sets and help doc, cmdlet review is https://github.com/Azure/azure-powershell-cmdlet-review-pr/issues/67

taiwu · 2018-05-07T19:24:59Z

@azuresdkci retest this please

praries880 · 2018-05-08T21:00:22Z

src/ResourceManager/AnalysisServices/ChangeLog.md

@@ -21,6 +21,7 @@

 ## Version 0.6.7
 * Set minimum dependency of module to PowerShell 5.0
+* Enable Gateway assocaite/disassociate operations on AS.


This should go directly under Current Release... We will move it to under a new version header when we generate the release version.

praries880 · 2018-05-08T21:02:19Z

src/ResourceManager/AnalysisServices/Commands.AnalysisServices.Test/ScenarioTests/AsTests.ps1

+        $resourceGroupName = Get-ResourceGroupName
+        $serverName = Get-AnalysisServicesServerName
+        $gatewayName = $env:GATEWAY_NAME
+        $gateway = Get-AzureRmResource -ResourceName $gatewayName -ResourceGroupName $resourceGroupName


So, this test assumes that the gateway server already exists in the subscription/RG... Is it possible to create the gateway server as part of the test?

Otherwise some one (lets say a person in the PS team) would not be able to re-record the test.

This have to be a pre-setup azure resource, which requires user to install unified gateway on their on-premise server and generate an gateway azure resource associated on it. It is not possible to automate the procedure in few lines of powershell scripts. This is LiveOnly test.

ok... can you add a comment to the test case that describes what all needs to be done for one to be able to set up the gateway server , the documentation could be as simple as pointing the person to follow an online guide on how to get it done as well ;) )?
That way the person running the test will have the instructions to do so....

praries880 · 2018-05-08T21:08:26Z

...ager/AnalysisServices/Commands.AnalysisServices/Commands/SetAzureRmAnalysisServicesServer.cs

-                AnalysisServicesServer updatedServer = AnalysisServicesClient.CreateOrUpdateServer(ResourceGroupName, Name, location, Sku, Tag, Administrator, currentServer, BackupBlobContainerUri, ReadonlyReplicaCount, DefaultConnectionMode, setting);
+                if (DisassociateGateway.IsPresent)
+                {
+                    GatewayResourceId = "-";


I would like to see this special value, namely "-" defined in a common file as a constant in some common codethat can be comsumed from the various places this is used.

praries880 · 2018-05-08T21:08:34Z

src/ResourceManager/AnalysisServices/Commands.AnalysisServices/Models/AnalysisServicesClient.cs

@@ -87,6 +88,16 @@ public AnalysisServicesClient()
                }
            }

+            GatewayDetails gatewayDetails = null;
+            if (gatewayResourceId == "-")


I would like to see this special value, namely "-" defined in a common file as a constant in some common codethat can be comsumed from the various places this is used.

taiwu · 2018-05-09T19:48:51Z

@markcowl Please take a look, the parameter sets are updated.

addressed.

maddieclayton · 2018-05-11T18:10:21Z

on demand here: https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/powershell-demand/831/

taiwu added 6 commits April 24, 2018 15:12

Enable gateway

c65f394

Merge branch 'taiwu-gw-march' into taiwu-gw-424

6666142

Fix indent and disassociate gateway

f578d45

Update change log

bdda3d5

Fix indent in test

86b1997

Fix indent in Set-operation

89febba

taiwu requested a review from maddieclayton April 25, 2018 00:25

azuresdkci added the needs-review label Apr 25, 2018

markcowl requested changes Apr 25, 2018

View reviewed changes

markcowl added Cmdlet Review Required ⚠️ Post Build and removed needs-review labels Apr 25, 2018

taiwu added 2 commits April 25, 2018 10:32

Update gateway association using resource id

ba8ef9d

Update help doc

db15652

markcowl removed the Post Build label May 1, 2018

markcowl previously requested changes May 1, 2018

View reviewed changes

markcowl assigned taiwu May 1, 2018

markcowl added the needs-revision label May 1, 2018

Update paramter sets for gateway operations and help doc

f098865

taiwu added 2 commits May 4, 2018 15:21

Merge branch 'preview' into taiwu-gw-424

d92e0c9

Fix md by removing trailing yaml

d9592f4

praries880 added Cmdlet Review Complete and removed Cmdlet Review Required ⚠️ labels May 8, 2018

praries880 suggested changes May 8, 2018

View reviewed changes

Fix changelog and minor changes for comments

f008839

praries880 added needs-review and removed needs-revision labels May 9, 2018

praries880 approved these changes May 9, 2018

View reviewed changes

maddieclayton added needs-revision and removed needs-review labels May 9, 2018

maddieclayton assigned praries880 and unassigned taiwu May 9, 2018

praries880 assigned markcowl and unassigned praries880 May 9, 2018

cormacpayne added needs-review and removed needs-revision labels May 10, 2018

maddieclayton approved these changes May 11, 2018

View reviewed changes

maddieclayton added Waiting for On Demand and removed Cmdlet Review Complete needs-review labels May 11, 2018

maddieclayton unassigned markcowl May 11, 2018

praries880 merged commit 17b6a03 into Azure:preview May 11, 2018

praries880 removed the Waiting for On Demand label May 11, 2018

[Azure Analysis Services] Enable associate/dissociate gateway with AS. #6043

[Azure Analysis Services] Enable associate/dissociate gateway with AS. #6043

Uh oh!

Conversation

taiwu commented Apr 25, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

taiwu commented Apr 26, 2018

Uh oh!

taiwu commented May 1, 2018

Uh oh!

markcowl commented May 1, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

taiwu commented May 1, 2018

Uh oh!

taiwu commented May 4, 2018

Uh oh!

taiwu commented May 7, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

praries880 May 8, 2018 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

taiwu commented May 9, 2018

Uh oh!

maddieclayton commented May 11, 2018

Uh oh!

Uh oh!

taiwu commented Apr 25, 2018 •

edited

Loading

praries880 May 8, 2018 •

edited

Loading