NFV-18043: modify cluster fields and add mgmt acl in existing device API#9
Conversation
I think we don't need to uphold consistency in this client with the interface provided in Terraform. We can continue the discussion about what we expect in our go types in #11. Other tools may depend on this interface and we don't want to introduce breaking changes each time we want to change Terraform, which operates under a different pattern for versioning. I've created #10 to address versioning in the future. Per our conversation, let's keep the existing fields and types and perhaps introduce the new fields in the existing types (or new types). |
The purpose of ne-go repository is for terraform provider development. We introduce client.go along with those types to further combine request and response class into one object so that other developers can better maintain it in our provider side code. The ne-go changes have to come first before any PRs sent for review in terraform provider repo. We have to find out a better way for this version management because it's common that we might make breaking changes in ne-go based on our discussion and need for the provider side changes. At that moment, PR is not merged at provider side but ne-go has been published. We face the dilemma where we cannot change ne-go because of the breaking change. However, we are the ones who introduce them in the first place and no one has consumed those changes at that time. There should be a way to publish temporary version for ne-go and after code changes are merged in provider side, we could mark this temporary version a final one. |
|
For this PR, those fields that are not required will be marked as deprecated to follow the versioning strategy here. |
…elds to match terraform
ocobles
left a comment
There was a problem hiding this comment.
I've left 1 comment. Everything else looks fine to me
| req.ClusterName = clusterDetails.ClusterName | ||
| req.NumOfNodes = clusterDetails.NumOfNodes | ||
| clusterNodeDetailsRequest := make(map[string]api.ClusterNodeDetailRequest) | ||
| for k, v := range clusterDetails.ClusterNodeDetails { |
There was a problem hiding this comment.
This for loop is no longer required since clusterNodeDetailsRequest["node0"] and clusterNodeDetailsRequest["node1"] is being defined below
This PR changes the format of
ClusterDetailsandClusterNodeDetail.ClusterNodeis no longer needed inclient.gobecause we will introduceNode0andNode1inClusterDetailswhile also removing the previous container of these fields,ClusterNodeDetails.This simplifies the structures and keeps
client.goconsistent with the Terraform changes being introduced in equinix/terraform-provider-equinix#105.