Added Quota functionality #8
Conversation
WapTenantPublicAPI.psm1
Outdated
| @@ -549,6 +549,192 @@ function GetWAPSubscriptionQuota { | |||
|
|
|||
| } | |||
|
|
|||
| function Get-WAPSubscriptionQuota{ | |||
There was a problem hiding this comment.
add space between function name and opening curly
Did you know there is already GetWAPSubscriptionQuota helper function? Could you investigate to what extent the logic is duplicate and if one can replace the other entirely or not?
There was a problem hiding this comment.
I did notice that. It seems to only be designed to work with SQL servers. I haven't got any of those yet so I can't outright test the function.
The internal function simply returns basequotasettings but only for SQL servers. It assumes you've selected a subscription.
My one collects subscription.service.basequotasettings.clouds.cloud.quota as well as calculating the current consumption. It takes Subscription Objects as inputs.
So they both have radically different views on what we are getting the Quota for (one is SQL, one is VM provisioning).
Possible options:
-
Expand the WAP.Quota object to include SQL so that if it's a SQL subscription, it includes the relavant parts.
-
Rename them to make the names clearer - perhaps Get-WAPSQLQuota and Get-WAPSubscriptionCloudQuota
Since the original helper function is never exported, we can be confident that renaming the old one won't dramatically break anything :)
WapTenantPublicAPI.psm1
Outdated
| PS C:\>Get-WAPSubscriptionQuota -Subscription $Sub | ||
| #> | ||
| [CmdletBinding()] | ||
| Param( |
WapTenantPublicAPI.psm1
Outdated
| [CmdletBinding()] | ||
| Param( | ||
| [Parameter( | ||
| Mandatory=$False, |
There was a problem hiding this comment.
Don't add Mandatory=$false. It's sufficient to not specify it if it's not mandatory ;).
If a Parameter is Mandatory, it should just have the word Mandatory without = $true
WapTenantPublicAPI.psm1
Outdated
| Param( | ||
| [Parameter( | ||
| Mandatory=$False, | ||
| ValueFromPipeline=$True, |
There was a problem hiding this comment.
Lowercase t on $True and there should be spaces surrounding '=' (everywhere ;) )
WapTenantPublicAPI.psm1
Outdated
| [Parameter( | ||
| Mandatory=$False, | ||
| ValueFromPipeline=$True, | ||
| ValueFromPipelineByPropertyName=$True, |
There was a problem hiding this comment.
What object has the property Subscription that can be bound by this attribute spec?
WapTenantPublicAPI.psm1
Outdated
| ForEach($Service in $Services){ | ||
| $BaseQuotaSettings = $Service.BaseQuotaSettings | ||
| ForEach($Setting in $BaseQuotaSettings){ | ||
| if($Setting.Key -eq "Clouds"){ |
WapTenantPublicAPI.psm1
Outdated
| [xml]$Data = $Setting.Value | ||
| $Output = $Data.Clouds.Cloud.Quota | ||
| $Object = New-Object -Type PsObject -Property @{ | ||
| SubscriptionName=$Sub.SubscriptionName; |
There was a problem hiding this comment.
Indent one level. You can replace the new-object by the [pscustomobject] type accelerator
WapTenantPublicAPI.psm1
Outdated
| PS C:\>Get-WAPVMRoleQuotaRequest -Subscription $Sub -VMRoleSizeProfile (Get-WAPVMRoleVMSize -Name A7) | ||
|
|
||
| #> | ||
| Param( |
There was a problem hiding this comment.
cmdletbinding missing and same lower case and indent comments as the other function
What is the default parametersetname?
WapTenantPublicAPI.psm1
Outdated
| ParameterSetName = "Size-Name" | ||
| )] | ||
| [string] | ||
| $Size, |
There was a problem hiding this comment.
Please construct parameter specifications similar to overall style
[type] $Name
WapTenantPublicAPI.psm1
Outdated
| [int] | ||
| $NumberOfNodes = 1 | ||
| ) | ||
| begin{ |
| @@ -53,7 +53,7 @@ function TestJWTClaimNotExpired { | |||
| if ($Token.split('.').count -ne 3) { | |||
| throw 'Invalid token passed, run Get-WAPToken to fetch a new one' | |||
| } | |||
| $TokenData = $token.Split('.')[1] | ForEach-Object -Process { | |||
| $TokenData = $token.Split('.')[1] | foreach-Object -Process { | |||
There was a problem hiding this comment.
ForEach-Object is a cmdlet. Should be PasCal case. I guess you did a search replace?
WapTenantPublicAPI.psm1
Outdated
| [CmdletBinding()] | ||
| param( | ||
| [Parameter( | ||
| ValueFromPipeline = $true, |
There was a problem hiding this comment.
No need to add $true
[Parameter(ValueFromPipeline, ValueFromPipelineByPropertyName)] is enough.
| ValueFromPipelineByPropertyName = $true | ||
| )] | ||
| [System.Management.Automation.PSTypeName('WAP.Subscription')] | ||
| [PSCustomObject] $Subscription = (Get-WAPSubscription) |
There was a problem hiding this comment.
You can remove the Datatype for this. So you'll end up with:
[Parameter(Attributes)]
[System.Management.Automation.PSTypeName('WAP.Subscription')] $Subscription
| Mandatory, | ||
| ValueFromPipeline = $true, | ||
| ValueFromPipelineByPropertyName = $true, | ||
| ParameterSetName = "Size-Object" |
There was a problem hiding this comment.
Some comments as above. If you use string literals (no variable expansion), please use single quotes.
Needed to be able to determine whether or not an API call would succeed due to quotas.
Get-WAPSubscriptionQuota pulls the quota and returns an object with nulls. Give it a Subscription - if there isn't one, it'll get the quota objects for all of them. You can use this for reporting purposes too.
Get-WAPVMRoleQuotaRequest takes a Subscription and VMRoleSizeProfile (or a name of one). It checks to see if the amount of those Node sizes will break the bank. Utilmately the WithinQuota boolean is what you need.
I've tested it and it all works, can you just do a quick code review to confirm you're happy @bgelens :)