clean up stackit client#1060
Conversation
ske-prow
Bot
added
the
kind/cleanup
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
ske-prow
Bot
added
needs-rebase
aniruddha2000
force-pushed
the
cleanup-stackit-client
branch
3 times, most recently
from
1819545 to
34cd84b
Compare
ske-prow
Bot
removed
the
needs-rebase
ske-prow
Bot
added
the
needs-rebase
# Conflicts: # go.mod # go.sum # Conflicts: # go.mod
aniruddha2000
force-pushed
the
cleanup-stackit-client
branch
from
391a765 to
9f381fc
Compare
ske-prow
Bot
removed
the
needs-rebase
| if err != nil { | ||
| klog.Fatalf("Failed to create IaaS client: %v", err) | ||
| iaasOpts := []sdkconfig.ConfigurationOption{ | ||
| sdkconfig.WithHTTPClient(metrics.NewInstrumentedHTTPClient()), // TODO: Ask if this is needed or not |
There was a problem hiding this comment.
What about the comment? I thin we need it
Did you test it?
| payload := iaas.ResizeVolumePayload{ | ||
| Size: volSizeGB, | ||
| } | ||
| err = cloud.ExpandVolume(ctx, volumeID, *volume.Status, payload) |
There was a problem hiding this comment.
| err = cloud.ExpandVolume(ctx, volumeID, *volume.Status, payload) | |
| err = cloud.ExpandVolume(ctx, volumeID, *volume.Status, iaas.ResizeVolumePayload{Size: volSizeGB}) |
Can we put this into one line?
| Do(func(_ context.Context, opts *iaas.CreateVolumePayload) { | ||
| iaasClient.EXPECT(). | ||
| CreateVolume(gomock.Any(), gomock.Any()). | ||
| DoAndReturn(func(_ context.Context, opts *iaas.CreateVolumePayload) (*iaas.Volume, error) { |
There was a problem hiding this comment.
Why was the Do changed into DoAndREturn?
| }, nil | ||
| } | ||
|
|
||
| func (i iaasClient) GetServer(ctx context.Context, serverID string) (*iaas.Server, error) { |
There was a problem hiding this comment.
Why is this a value receiver instead of a pointer reviewer?
func (i *iaasClient) vs
func (i iaasClient) | }, nil | ||
| } | ||
|
|
||
| func (l loadBalancingClient) CreateLoadBalancer(ctx context.Context, payload *loadbalancer.CreateLoadBalancerPayload) (*loadbalancer.LoadBalancer, error) { |
There was a problem hiding this comment.
Why the value receiver? See above IaaSClient
There was a problem hiding this comment.
We should also here add the X-Request-Id header copy to error message wrapper.
I would prefer however if we can find a way to make this prettier than individually wrapping each call.
2 participants
How to categorize this PR?
/kind cleanup
What this PR does / why we need it:
This PR introduces clean up for the stackit client and the mocks.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Breaking changes: