Azure powershell SDK changes for Notification Hubs. Added a skuTier parameter for create/update namespace. #3119

amolr · 2016-10-25T17:33:46Z

Description

The create/update namespace operation for Notification Hubs can now use a SkuTier parameter which sets the Sku level at the namespace level. This change allows the user to set the Sku using the parameter which creating or updating the namespace. In case of create, the skuTier if not specified is free by default.

This checklist is used to make sure that common guidelines for a pull request are followed. You can find a more complete discussion of PowerShell cmdlet best practices here.

I have read the contribution guidelines.
**If changes were made to any cmdlet, the XML help was regenerated using the Powershell CmdLet Help Editor
If any large changes are made to a service, they are reflected in the respective change log.

General Guidelines

Title of the pull request is clear and informative.
There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.
The pull request does not introduce breaking changes (unless a major version change occurs in the assembly and module).

Testing Guidelines

Pull request includes test coverage for the included changes.
PowerShell scripts used in tests should do any necessary setup as part of the test or suite setup, and should not use hard-coded values for locations or existing resources.

azurecla · 2016-10-25T17:33:57Z

Hi @amolr, I'm your friendly neighborhood Azure Pull Request Bot (You can call me AZPRBOT). Thanks for your contribution!

It looks like you're working at Microsoft (amolr). If you're full-time, we DON'T require a contribution license agreement.

If you are a vendor, DO please sign the electronic contribution license agreement. It will take 2 minutes and there's no faxing! https://cla.azure.com.

TTYL, AZPRBOT;

azuresdkci · 2016-10-25T17:35:21Z

Can one of the admins verify this patch?

azuresdkci · 2016-11-04T21:45:03Z

Can one of the admins verify this patch?

cormacpayne · 2016-11-07T17:50:06Z

@azuresdkci add to whitelist

shahabhijeet · 2016-11-09T18:58:41Z

@amolr can you address failures

cormacpayne · 2016-11-10T23:33:43Z

@amolr Hey Amol, can you add the following tests:

Using New-AzureRmNotificationHubsNamespace where the SkuTier is "Free" or isn't used at all
Using Set-AzureRmNotificationHubsNamespace where the SkuTier is a value other than "Free"

cormacpayne · 2016-11-16T21:59:50Z

...anager/NotificationHubs/Commands.NotificationHubs.Test/Commands.NotificationHubs.Test.csproj

      <Private>True</Private>
    </Reference>
    <Reference Include="Microsoft.Azure.ResourceManager, Version=2.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
      <HintPath>..\..\..\packages\Microsoft.Azure.Management.Resources.2.20.0-preview\lib\net40\Microsoft.Azure.ResourceManager.dll</HintPath>
      <Private>True</Private>
    </Reference>
-    <Reference Include="Microsoft.Azure.Test.Framework">
+    <Reference Include="Microsoft.Azure.Test.Framework, Version=1.0.6052.28118, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">


@amolr why was this change made?

I think this change got introduced while trying to debug an issue with Abhijeet related to our tests failing

cormacpayne · 2016-11-16T22:01:08Z

...cationHubs/Commands.NotificationHubs/Commands/Namespace/NewAzureNotificationHubsNamespace.cs

+            ValueFromPipelineByPropertyName = true,
+            Position = 4,
+            HelpMessage = "Sku tier of the namespace")]
+        public string skuTier { get; set; }


@amolr please rename this to SkuTier

cormacpayne · 2016-11-16T22:01:22Z

...cationHubs/Commands.NotificationHubs/Commands/Namespace/SetAzureNotificationHubsNamespace.cs

+          ValueFromPipelineByPropertyName = true,
+          Position = 4,
+          HelpMessage = "Sku tier of the namespace")]
+        public string skuTier { get; set; }


@amolr please rename this to SkuTier

cormacpayne · 2016-11-16T22:12:14Z

...cationHubs/Commands.NotificationHubs/Commands/Namespace/SetAzureNotificationHubsNamespace.cs

@@ -78,7 +84,7 @@ public override void ExecuteCmdlet()
                () =>
                {
                    // Update a namespace 
-                    var nsAttribute = Client.UpdateNamespace(ResourceGroup, Namespace, Location, State, Critical, ConvertTagsToDictionary(Tags));
+                    var nsAttribute = Client.UpdateNamespace(ResourceGroup, Namespace, Location, State, Critical, ConvertTagsToDictionary(Tags), skuTier);


@amolr does SkuTier need to have a default value here like it does in New-AzureNotificationHubsNamespace? For example, if someone passes in an empty string for SkuTier, does it need to be set to "free"?

No it need not have it here

cormacpayne · 2016-11-17T18:53:57Z

@amolr please update the change log with the changes you have made in this PR

markcowl · 2016-11-22T00:59:44Z

@amolr Please update the changelog as Cormac suggested, here: https://github.com/Azure/azure-powershell/blob/dev/src/ResourceManager/NotificationHubs/ChangeLog.md

Also, please clean up the commits, you have several commits titled 'regenerate wxi file'.. There are instructions for cleaning up commits here: https://github.com/Azure/azure-powershell/blob/dev/documentation/cleaning-up-commits.md

markcowl · 2016-11-22T19:54:31Z

@amolr still looking for commit cleanup. Please remove the '[Cleup commits]' tag from the title when you have cleaned up the commits.

…lp file

cormacpayne · 2016-11-23T23:06:15Z

LGTM

azurecla added the cla-not-required label Oct 25, 2016

shahabhijeet assigned cormacpayne Nov 9, 2016

cormacpayne suggested changes Nov 16, 2016

View reviewed changes

markcowl changed the title ~~Azure powershell SDK changes for Notification Hubs. Added a skuTier parameter for create/update namespace.~~ [Cleanup commite]:Azure powershell SDK changes for Notification Hubs. Added a skuTier parameter for create/update namespace. Nov 22, 2016

markcowl changed the title ~~[Cleanup commite]:Azure powershell SDK changes for Notification Hubs. Added a skuTier parameter for create/update namespace.~~ [Cleanup commits]:Azure powershell SDK changes for Notification Hubs. Added a skuTier parameter for create/update namespace. Nov 22, 2016

Added Sku parameter and changed tests

055aaf0

amolr force-pushed the dev branch from 73f9ce9 to 055aaf0 Compare November 23, 2016 01:14

amolr changed the title ~~[Cleanup commits]:Azure powershell SDK changes for Notification Hubs. Added a skuTier parameter for create/update namespace.~~ Azure powershell SDK changes for Notification Hubs. Added a skuTier parameter for create/update namespace. Nov 23, 2016

cormacpayne approved these changes Nov 23, 2016

View reviewed changes

Removed <Private>true</Private> and changing skuTier to SkuTier in he…

80cd89a

…lp file

cormacpayne merged commit 626036a into Azure:dev Nov 23, 2016

Azure powershell SDK changes for Notification Hubs. Added a skuTier parameter for create/update namespace. #3119

Azure powershell SDK changes for Notification Hubs. Added a skuTier parameter for create/update namespace. #3119

Uh oh!

Conversation

amolr commented Oct 25, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

azurecla commented Oct 25, 2016

Uh oh!

azuresdkci commented Oct 25, 2016

Uh oh!

azuresdkci commented Nov 4, 2016

Uh oh!

cormacpayne commented Nov 7, 2016

Uh oh!

shahabhijeet commented Nov 9, 2016

Uh oh!

cormacpayne commented Nov 10, 2016

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!

cormacpayne commented Nov 17, 2016

Uh oh!

markcowl commented Nov 22, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

markcowl commented Nov 22, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cormacpayne commented Nov 23, 2016

Uh oh!

Uh oh!

amolr commented Oct 25, 2016 •

edited

Loading

markcowl commented Nov 22, 2016 •

edited

Loading

markcowl commented Nov 22, 2016 •

edited

Loading