Review Get-vRAResource function #171
Conversation
Create an inner function New-vRAObjectResource for centralizing vRA Resource PS Object management Fix URI when Resource ID is used Fix URI when Resource Name is used (vRA 7 call is case sensitive for the resource name) Add pages management when no parameter is passed. Before, only the first page was returned (so 100 items).
| Links = $Resource.links | ||
| IconId = $Resource.iconId | ||
|
|
||
| if ($Response.content.Count -ne 0) { |
There was a problem hiding this comment.
Is this a typo? Should it be:
$Resource.content.Count
| @@ -116,36 +116,17 @@ | |||
|
|
|||
| foreach ($ResourceId in $Id) { | |||
|
|
|||
| $URI = "/catalog-service/api/consumer/resourceViews/$($ResourceId)?withExtendedData=true&withOperations=true" | |||
| $URI = "/catalog-service/api/consumer/resourceViews?`$filter=id eq '$($ResourceId)'&withExtendedData=true&withOperations=true" | |||
|
|
|||
| $EscapedURI = [uri]::EscapeUriString($URI) | |||
|
|
|||
| $Resource = Invoke-vRARestMethod -Method GET -URI $EscapedURI -Verbose:$VerbosePreference | |||
There was a problem hiding this comment.
My bad @jonathanmedd , I forgot to rename $Resource to $Response here in your version of the script.
There was a problem hiding this comment.
OK. Can you just update that. I think the rest of the PR is fine
Fix a mistaken variable declaration.
| } | ||
|
|
||
| Function New-vRAObjectResource { |
There was a problem hiding this comment.
At the moment, our build script will export this as a public function. Could you modify the name to be:
function intNewvRAObjectResource
See here for reference:
Also we tend to use [PSCustomObject] for building objects. Could you change line 249 - 274 to:
[PSCustomObject]@{
ResourceId = $Data.ResourceId
BusinessGroupId = $Data.businessGroupId
BusinessGroupName = $Data.data.MachineGroupName
TenantId = $Data.tenantId
CatalogItemLabel = $Data.data.Component
ParentResourceId = $Data.parentResourceId
HasChildren = $Data.hasChildren
Data = $Data.data
ResourceType = $Data.resourceType
Name = $Data.name
Description = $Data.description
Status = $Data.status
RequestId = $Data.requestId
Owners = $Data.owners
DateCreated = $Data.dateCreated
LastUpdated = $Data.lastUpdated
Lease = $Data.lease
Costs = $Data.costs
CostToDate = $Data.costToDate
TotalCost = $Data.totalCost
Links = $Data.links
IconId = $Data.iconId
}
See this function for reference:
There was a problem hiding this comment.
Ok for renaming the function of course.
About your remark about typing the object, the New-Object -TypeName PSObject cmdlet already type the object as PSCustomObject.
And as I use splatting to dynamically set object properties, I think it's not necessary.
What's your point?
There was a problem hiding this comment.
@BlackCatDeployment It's more performant to use the [pscustomobject] way to create objects, rather than using New-Object. (See https://www.jonathanmedd.net/2011/09/powershell-v3-creating-objects-with-pscustomobject-its-fast.html for a rough comparison of speed difference) Also, it would be more consistent with how we have created objects in the rest of the project code.
There was a problem hiding this comment.
Ok done :)
|
@BlackCatDeployment Thanks so much for your contribution and apologies that it's taken me so long to review the PR. I've left a couple of comments above. Once those are sorted we can merge and this will be included in the next release!! Thanks 👍 🥇 |
To meet PowervRA dev syntax, renamed function New-vRAObjectResource to intNewvRAObjectResource
Review PSObject initialization to meet PowervRA dev syntax.
Create an inner function New-vRAObjectResource for centralizing vRA Resource PS Object management
Fix URI when Resource ID is used
Fix URI when Resource Name is used (vRA 7 call is case sensitive for the resource name)
Add pages management when no parameter is passed. Before, only the first page was returned (so 100 items).