feat(bigtable): support partial results in InstanceAdminClient.Cluste… #2932
Conversation
IlyaFaer
added
type: feature request
google-cla
bot
added
the
cla: yes
|
@kolea2 can you confirm that this makes sense to add from a product perspective? If so I will give it a review. |
| @@ -1034,6 +1036,11 @@ func (iac *InstanceAdminClient) Clusters(ctx context.Context, instanceID string) | |||
| StorageType: storageTypeFromProto(c.DefaultStorageType), | |||
| }) | |||
| } | |||
| if len(res.FailedLocations) > 0 { | |||
There was a problem hiding this comment.
Hm-m, as we're not very able to simulate "failed locations" error on real data (at least I don't know how this can be done), I think a test can be written with kind of a mock for ListClusters() method. But the mock is big, as there is an interface implementation restriction, and the interface describes a lot of methods. By the way, I see that emulator doesn't support most part of instance and cluster related methods. Should we implement them? Doesn't seem to be very hard.
| @@ -1034,6 +1036,11 @@ func (iac *InstanceAdminClient) Clusters(ctx context.Context, instanceID string) | |||
| StorageType: storageTypeFromProto(c.DefaultStorageType), | |||
| }) | |||
| } | |||
| if len(res.FailedLocations) > 0 { | |||
There was a problem hiding this comment.
Hm-m, as we're not very able to simulate "failed locations" error on real data (at least I don't know how this can be done), I think a test can be written with kind of a mock for ListClusters() method. But the mock is big, as there is an interface implementation restriction, and the interface describes a lot of methods. By the way, I see that emulator doesn't support most part of instance and cluster related methods. Should we implement them? Doesn't seem to be very hard.
| FailedLocations: iacm.UnavailableLocations, | ||
| } | ||
| return &res, nil | ||
| } |
There was a problem hiding this comment.
This method will be called by Clusters which I'm changing with this PR.
| iAdminClient.iClient = &instanceAdminClientMock{ | ||
| Clusters: []*btapb.Cluster{&cluster1}, | ||
| UnavailableLocations: []string{failedLoc}, | ||
| } |
There was a problem hiding this comment.
Setting mock data for the test.
|
LGTM. Will give @kolea2 some time and merge tomorrow either way. |
🤖 I have created a release \*beep\* \*boop\* --- ## [1.8.0](https://www.github.com/googleapis/google-cloud-go/compare/v1.7.1...v1.8.0) (2021-02-24) ### Features * **bigtable:** support partial results in InstanceAdminClient.Clusters() ([#2932](https://www.github.com/googleapis/google-cloud-go/issues/2932)) ([28decb5](https://www.github.com/googleapis/google-cloud-go/commit/28decb55c366c5ec67e04800aa06179943b765f6)) This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
…rs()
Towards #2914