Upgrade parameters on Cmdlet Get-AzureRmConsumptionUsageDetail #6023

bgsky · 2018-04-23T21:37:39Z

Description

Depreciated parameter InvoiceName on Cmdlet Get-AzureRmConsumptionUsageDetail
Add new parameter Expand, ResourceGroup, InstanceName, InstanceId, Tag, and Top on Cmdlet Get-AzureRmConsumptionUsageDetail

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

azuresdkci · 2018-04-23T21:41:02Z

Can one of the admins verify this patch?

bgsky · 2018-04-24T21:26:51Z

@azuresdkci Just joined Azure, please verify the status.

maddieclayton · 2018-04-24T21:50:02Z

@azuresdkci Add to whitelist

…nto preview

markcowl · 2018-05-01T23:10:20Z

@bgsky Please fill out a cmdlet design review here: https://github.com/Azure/azure-powershell-cmdlet-review-pr/issues

…nto preview

bgsky · 2018-05-02T18:20:34Z

@markcowl Please refer to issue request
https://github.com/Azure/azure-powershell-cmdlet-review-pr/issues/63

…nto preview

maddieclayton · 2018-05-04T20:37:02Z

src/ResourceManager/Consumption/Commands.Consumption.Test/Commands.Consumption.Test.csproj

@@ -84,6 +85,14 @@
      <SpecificVersion>False</SpecificVersion>
      <HintPath>..\..\..\packages\Microsoft.IdentityModel.Clients.ActiveDirectory.2.28.3\lib\net45\Microsoft.IdentityModel.Clients.ActiveDirectory.WindowsForms.dll</HintPath>
    </Reference>
+    <Reference Include="Microsoft.Rest.ClientRuntime, Version=2.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">


Remove these two references, as it is included in the common targets.

Thanks for pointing out. Removed.

maddieclayton · 2018-05-04T20:37:11Z

src/ResourceManager/Consumption/Commands.Consumption.Test/Commands.Consumption.Test.csproj

-      <HintPath>..\..\..\packages\Microsoft.Azure.Management.Consumption.1.1.0-preview\lib\net452\Microsoft.Azure.Management.Consumption.dll</HintPath>
+    <Reference Include="Microsoft.Azure.Management.Consumption, Version=3.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
+      <HintPath>..\..\..\packages\Microsoft.Azure.Management.Consumption.3.0.0\lib\net452\Microsoft.Azure.Management.Consumption.dll</HintPath>
+      <Private>True</Private>


Why do you need this private tag?

This tag was added by accident. Already removed.

maddieclayton · 2018-05-04T20:39:53Z

src/ResourceManager/Consumption/Commands.Consumption.Test/ScenarioTests/UsageDetailsTests.ps1

-    $usageDetails = Get-AzureRmConsumptionUsageDetail -MaxCount 10
-
+    $usageDetails = Get-AzureRmConsumptionUsageDetail -Top 10
+	Assert-NotNull $usageDetails


Do these tests only work on a specific subscription?

No, we have the same internal implementation for MaxCount and Top.

maddieclayton · 2018-05-04T20:40:18Z

src/ResourceManager/Consumption/Commands.Consumption.Test/packages.config

@@ -14,6 +14,8 @@
  <package id="Microsoft.Bcl.Build" version="1.0.14" targetFramework="net45" />
  <package id="Microsoft.IdentityModel.Clients.ActiveDirectory" version="2.28.3" targetFramework="net45" />
  <package id="Microsoft.Net.Http" version="2.2.28" targetFramework="net45" />
+  <package id="Microsoft.Rest.ClientRuntime" version="2.3.10" targetFramework="net452" />


Remove these two packages as well.

Thanks for pointing out. Removed.

maddieclayton · 2018-05-04T20:41:02Z

...er/Consumption/Commands.Consumption/Cmdlets/UsageDetails/GetAzureRmConsumptionUsageDetail.cs

+
+        [Parameter(Mandatory = false, HelpMessage = "The tag of the usages to filter.")]
+        [ValidateNotNull]
+        public string Tags { get; set; }


Rename to "Tag" (powershell guidelines)

Tags is renamed as Tag, and .md file and recorded sessions are regenerated accordingly.

maddieclayton · 2018-05-04T20:41:51Z

...er/Consumption/Commands.Consumption/Cmdlets/UsageDetails/GetAzureRmConsumptionUsageDetail.cs

+
+        [Parameter(Mandatory = false, HelpMessage = "Name of a specific invoice to get the usage details that associate with.")]
+        [ValidateNotNullOrEmpty]
+        public string InvoiceName { get; set; }


Add obsolete tag on this parameter: https://github.com/Azure/azure-powershell/blob/preview/documentation/breaking-changes/breaking-changes-attribute-help.md

Added an attribute of CmdletParameterBreakingChange on InvoiceName.

maddieclayton · 2018-05-04T20:44:05Z

src/ResourceManager/Consumption/Commands.Consumption/Commands.Consumption.csproj

-      <HintPath>..\..\..\packages\Microsoft.Azure.Management.Consumption.1.1.0-preview\lib\net452\Microsoft.Azure.Management.Consumption.dll</HintPath>
+    <Reference Include="Microsoft.Azure.Management.Consumption, Version=3.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
+      <HintPath>..\..\..\packages\Microsoft.Azure.Management.Consumption.3.0.0\lib\net452\Microsoft.Azure.Management.Consumption.dll</HintPath>
+      <Private>True</Private>


Why the private tag?

Added by accident. Already removed.

maddieclayton · 2018-05-04T20:45:26Z

src/ResourceManager/Consumption/Commands.Consumption/Models/PSUsageDetail.cs

        public string Id { get; private set; }
-        public string Name { get; private set; }


Renaming properties is a breaking change - please add back InvoiceName and BillingPeriodName

Sorry for confusion of shuffling the parameters on alphabetic order. InvoiceName and BillingPeriodName were added back.

maddieclayton · 2018-05-04T20:48:00Z

src/ResourceManager/Consumption/Commands.Consumption/help/Get-AzureRmConsumptionUsageDetail.md

 ```

-Get usage details of the invoice with specified name, and include MeterDetails property in the result.


Please provide example output for these examples.

@maddieclayton Our model of usage detail has 28 output parameters. Can we skip output example to avoid user distraction?

maddieclayton · 2018-05-04T20:49:04Z

src/ResourceManager/Consumption/Commands.Consumption/help/Get-AzureRmConsumptionUsageDetail.md

+Accept wildcard characters: False
+```
+
+### -Tags


Be sure to rengenerate the file after changing Tags to Tag to reflect the change.

.md file is regenerated accordingly.

Looks like it is still tags

Change it to tag

…sing parameters in PSUsage model

…nto preview

bgsky · 2018-05-05T00:07:43Z

@maddieclayton @ms-premp We addressed your comments on Tag, redundant csproj references, depreciated InvoiceName attribute, missing parameters in PSUsageDetail model. Please review the latest iteration. Pull request description is also updated with the latest changes.

maddieclayton · 2018-05-08T16:55:13Z

src/ResourceManager/Consumption/ChangeLog.md

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

 ## Version 0.3.2
 * Set minimum dependency of module to PowerShell 5.0
+* Add new parameters Expand, ResourceGroup, InstanceName, InstanceId, Tags, and Top on Cmdlet Get-AzureRmConsumptionUsageDetail


Move this to "Current Release"

Will move to Current Release

maddieclayton · 2018-05-08T16:59:30Z

src/ResourceManager/Consumption/Commands.Consumption/Models/PSUsageDetail.cs

+        public string Product { get; set; }
+        public string SubscriptionGuid { get; set; }
+        public string SubscriptionName { get; set; }
+        public IDictionary<string, string> Tag { get; private set; }


Keep this one as "Tags" because otherwise it is a breaking change.

Will keep it as Tags

maddieclayton · 2018-05-08T17:01:28Z

src/ResourceManager/Consumption/Commands.Consumption/help/Get-AzureRmConsumptionUsageDetail.md

+Accept wildcard characters: False
+```
+
+### -Tags


Looks like it is still tags

maddieclayton · 2018-05-08T17:04:45Z

src/ResourceManager/Consumption/Commands.Consumption/Models/PSUsageDetail.cs

-        public DateTime? UsageStart { get; private set; }
-        public DateTime? UsageEnd { get; private set; }
+        public string AccountName { get; set; }
+        public string AdditionalProperties { get; set; }


Looks like this changed type - please change back.

@maddieclayton @ms-premp As we need this output as string, we keep AdditionalProperties as dictionary type (will depreciate it and label as future breaking change), and introduce a new output parameter as AdditionalInfo with string type.

…essions

…nto preview

bgsky · 2018-05-08T21:19:52Z

@maddieclayton @ms-premp The new iteration fixed typo on Tag parameter. In terms of a parameter of AdditionalProperties, we need string type, therefore keep it as dictionary type, and introduce a new parameter AdditionalInfo as string type. Recorded test sessions are also regenerated with those changes. Please review and let me know any comments

maddieclayton · 2018-05-08T21:24:48Z

src/ResourceManager/Consumption/Commands.Consumption/Models/PSUsageDetail.cs

-                this.UsageStart = usageDetail.UsageStart;
-                this.UsageEnd = usageDetail.UsageEnd;
+                this.AccountName = usageDetail.AccountName;
+                this.AdditionalInfo = usageDetail.AdditionalProperties;


What was AdditionalProperties before? It seems like now a property that was previously populated will always be null. Is it possible to populate this value?

It will always be null. AdditionalProperties of dictionary type was developed by previous team long ago. We don't have information on how to populate it in a dictionary type. Instead we do AdditionInfo in string type, and mark AdditionalProperties as future breaking change. Let me know your advice in that case.

As long as the current behavior is that it always returns null, it is fine that it now will not be populated.

maddieclayton · 2018-05-08T22:40:44Z

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

REDMOND\chanwan added 3 commits April 23, 2018 14:03

Upgraded usage command on consumption

3c6864b

Removed commented code on GetAzureRmConsumptionUsageDetail

7cff053

Add change comments on md files

809071c

markcowl added the Cmdlet Review Required ⚠️ label Apr 23, 2018

azuresdkci added the needs-review label Apr 23, 2018

Update .Netcore package reference

3fe6354

markcowl added the Post Build label Apr 25, 2018

REDMOND\chanwan added 2 commits April 25, 2018 17:07

Redesigned parameters on usage Cmdlet to mitigate break changes

64cbe45

Merge branch 'preview' of https://github.com/Azure/azure-powershell i…

36a17cd

…nto preview

markcowl removed the Post Build label May 1, 2018

markcowl assigned bgsky May 1, 2018

markcowl removed the needs-review label May 1, 2018

REDMOND\chanwan added 2 commits May 2, 2018 11:14

Removed current break change and set as upcoming

326e908

Merge branch 'preview' of https://github.com/Azure/azure-powershell i…

8ca0304

…nto preview

REDMOND\chanwan added 2 commits May 4, 2018 11:09

Add usage detail markdown file

0e78c01

Merge branch 'preview' of https://github.com/Azure/azure-powershell i…

42b3974

…nto preview

maddieclayton added Cmdlet Review Complete needs-review and removed Cmdlet Review Required ⚠️ labels May 4, 2018

maddieclayton assigned maddieclayton and unassigned bgsky May 4, 2018

maddieclayton suggested changes May 4, 2018

View reviewed changes

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

maddieclayton removed their assignment May 4, 2018

REDMOND\chanwan added 2 commits May 4, 2018 16:42

Addressed comments on Tag, csproj files, depreciated InvoiceName, mis…

1f7aa58

…sing parameters in PSUsage model

Merge branch 'preview' of https://github.com/Azure/azure-powershell i…

dd70cd1

…nto preview

bgsky added this to the 2018-05-18 milestone May 4, 2018

bgsky added needs-review and removed needs-revision labels May 5, 2018

maddieclayton self-assigned this May 5, 2018

cormacpayne unassigned bgsky May 7, 2018

maddieclayton suggested changes May 8, 2018

View reviewed changes

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

cormacpayne assigned bgsky and unassigned maddieclayton May 8, 2018

REDMOND\chanwan added 2 commits May 8, 2018 14:10

Addressed comments on parameter type and typo, and regenerated test s…

82463e5

…essions

Merge branch 'preview' of https://github.com/Azure/azure-powershell i…

35468ac

…nto preview

maddieclayton reviewed May 8, 2018

View reviewed changes

bgsky assigned maddieclayton and unassigned bgsky May 8, 2018

bgsky added needs-review and removed needs-revision labels May 8, 2018

maddieclayton approved these changes May 8, 2018

View reviewed changes

maddieclayton added Waiting for On Demand and removed needs-review labels May 8, 2018

maddieclayton removed their assignment May 8, 2018

This was referenced May 8, 2018

Update to most current WebApi for Get-AzureRmConsumptionUsageDetail #5444

Closed

Get-AzureRmConsumptionUsageDetail missing page results #5631

Closed

MiYanni merged commit 0836bde into Azure:preview May 9, 2018

		public string Id { get; private set; }
		public string Name { get; private set; }

		```

		Get usage details of the invoice with specified name, and include MeterDetails property in the result.

Upgrade parameters on Cmdlet Get-AzureRmConsumptionUsageDetail #6023

Upgrade parameters on Cmdlet Get-AzureRmConsumptionUsageDetail #6023

Uh oh!

Conversation

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

Uh oh!

Description

Checklist

Uh oh!

azuresdkci commented Apr 23, 2018

Uh oh!

bgsky commented Apr 24, 2018

Uh oh!

maddieclayton commented Apr 24, 2018

Uh oh!

markcowl commented May 1, 2018

Uh oh!

bgsky commented May 2, 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!

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!

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!

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!

Choose a reason for hiding this comment

Uh oh!

bgsky commented May 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

bgsky commented Apr 23, 2018 •

edited

Loading

bgsky commented May 5, 2018 •

edited

Loading