Improve Compute Manager data source #976
Conversation
Since usually only one compute manager is configured, it is useful to retrieve its ID without providing any filters. This PR enables this. Signed-off-by: Anna Khmelnitsky <[email protected]>
|
/test-all |
| @@ -57,6 +55,11 @@ func dataSourceNsxtComputeManagerRead(d *schema.ResourceData, m interface{}) err | |||
| var perfectMatch []model.ComputeManager | |||
| var prefixMatch []model.ComputeManager | |||
| for _, objInList := range objList.Results { | |||
| if len(objName) == 0 { | |||
| // We want to grab single compute manager | |||
| perfectMatch = append(perfectMatch, objInList) | |||
There was a problem hiding this comment.
So if there are multiple compute managers, an empty display_name will cause an error. I'm wondering if we should add a more informative err msg indicating empty display_name can only be used with single compute manager to reduce the confusion or change the behavior here to just grab the first compute manager or just treat it as a match all for prefixMatch. What do you think?
There was a problem hiding this comment.
I don't think we should decide which manager to grab without further input from the user.
Failing the data source because multiple objects are matching is a behavior present in most data sources in the provider. Regarding error messages - perhaps we should unify these messages across all data sources for consistency?
I can add more info in the doc for the case of compute manager.
|
|
||
| resource.ParallelTest(t, resource.TestCase{ | ||
| PreCheck: func() { | ||
| testAccOnlyLocalManager(t) |
There was a problem hiding this comment.
Not sure whether we need testAccTestMP() for these resources or not. Other than this LGTM
There was a problem hiding this comment.
I think we should rename testAccTestMP to testAccTestDeprecated. I think we should include the new fabric resources in the main test suite
There was a problem hiding this comment.
Makes sense. I think that I added testAccTestMP to a some fabric resource(s) which I coded. I'll remove these and rename testAccTestMP
|
@annakhm, you must sign every commit in this pull request acknowledging our Developer Certificate of Origin before your changes are merged. This can be done by adding
|
Since usually only one compute manager is configured, it is useful to retrieve its ID without providing any filters. This PR enables this.