Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extended the InventoryBaseDSC class to resolve the path to a specific Inventory Item #90

Merged
merged 6 commits into from
Feb 13, 2019

Conversation

SimeonGerginov
Copy link
Contributor

Signed-off-by: SimeonGerginov simeongerginov1@gmail.com

Signed-off-by: SimeonGerginov <simeongerginov1@gmail.com>
@SimeonGerginov SimeonGerginov added the enhancement New feature or request label Feb 6, 2019
Signed-off-by: SimeonGerginov <simeongerginov1@gmail.com>
Signed-off-by: SimeonGerginov <simeongerginov1@gmail.com>
Copy link
Contributor

@dmilov dmilov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sure there is more effective way to resolve path. Now algorithm starts with the parent folder, gets all children on the next level from the server and filters by name client-side and this repeat to the last level in the path. Those are a lot of server requests.

One way to optimize is getting inventory item by name, and going up through it's parent names to check which on if any has the desired path. I think the API allows filtering by name and you can request only the name property which will reduce the server round-trips and the data size retrieved from the server. I think Luc was implemented something similar in his resources. You can talk to him about it.

The request here is spend a little bit more time to figure out whether ammount of data and server round trips could be optimized before going forward with the resources that will inherit this base.

@SimeonGerginov SimeonGerginov changed the title Added HostInventoryBaseDSC class Extended the InventoryBaseDSC class to resolve the path to a specific Inventory Item Feb 8, 2019
Signed-off-by: SimeonGerginov <simeongerginov1@gmail.com>
@SimeonGerginov
Copy link
Contributor Author

Tested with the following values:

DatacenterPath = Datacenter
InventoryItemPath = host/MyNewFolder1/MyNewFolder2/MyNewFolder3/MyNewFolder4/MyTestCluster
InventoryItemName = Resources

Copy link
Contributor

@dmilov dmilov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Rename "InventoryItemName" property to "Name"
  2. Rename "InventoryItemPath" property to "InventoryPath"
  3. Rename "DatacenterPath" property to "Datacenter"
  4. InventoryPath shouldn't include the root folder. Define hidden property that will be set by inheritors to determine the root folder.

Here is an example configuration for reference

MyDatacenterFolder
-MyDatacenter
--MyCluster
---MyHost1
---MyDVS
---MyVM

?Datacenter MyDatacetner {
Name =
Path =
}

Cluster MyCluster {
[Key]Name = "MyCluster"
[Key]InventoryPath = ""
[Key]Datacenter = "MyDatacenterFolder/MyDatacenter"
DependsOn = [Datacenter]
Ensure = Present
}

DVS MyDVS {
Name = "MyDVS"
InvtoryPath = "MyNetworkFolder"
DatacenterPath = "MyDatacenterFolder/MyDatacenter"
Ensure = Present
}

VM MyVM {
Name = "MyVM"
InvtoryPath = "Discovered Virtual Machines"
DatacenterPath = "MyDatacenterFolder/MyDatacenter"
?Datastore = MyDatastore1
?Compute = MyHost1
Ensure = Present
DependsOn = [MyDatastore1]
DependsOn = [MyHost1]
}

Signed-off-by: SimeonGerginov <simeongerginov1@gmail.com>
Copy link
Contributor

@dmilov dmilov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update the documentation comments per my review comments. Everything else looks good.

Source/VMware.vSphereDSC/Classes/InventoryBaseDSC.ps1 Outdated Show resolved Hide resolved
Source/VMware.vSphereDSC/Classes/InventoryBaseDSC.ps1 Outdated Show resolved Hide resolved
Source/VMware.vSphereDSC/Classes/InventoryBaseDSC.ps1 Outdated Show resolved Hide resolved
Source/VMware.vSphereDSC/Classes/InventoryBaseDSC.ps1 Outdated Show resolved Hide resolved
Source/VMware.vSphereDSC/Classes/InventoryBaseDSC.ps1 Outdated Show resolved Hide resolved
Source/VMware.vSphereDSC/Classes/InventoryBaseDSC.ps1 Outdated Show resolved Hide resolved
Source/VMware.vSphereDSC/Classes/InventoryBaseDSC.ps1 Outdated Show resolved Hide resolved
Source/VMware.vSphereDSC/Classes/InventoryBaseDSC.ps1 Outdated Show resolved Hide resolved
Signed-off-by: SimeonGerginov <simeongerginov1@gmail.com>
@dmilov dmilov merged commit 871d0ed into vmware:master Feb 13, 2019
@SimeonGerginov SimeonGerginov deleted the inventory branch February 13, 2019 11:11
@SimeonGerginov SimeonGerginov added this to the 2.0 milestone Apr 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants