Replace deprecated aws session.New() with session.NewSession() #3255
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
|
Hi @alrs. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
k8s-ci-robot
added
the
needs-ok-to-test
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: alrs Assign the PR to them by writing The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
|
/assign @chrislovecnm |
|
/ok-to-test |
k8s-ci-robot
removed
the
needs-ok-to-test
| c.cf = cloudformation.New(session.New(), config) | ||
| sess, err := session.NewSession() | ||
| if err != nil { | ||
| return &awsCloudImplementation{}, err |
There was a problem hiding this comment.
Dumb question but why are we returning a new struct?
There was a problem hiding this comment.
No good reason, so I've changed it to use the extant "c" variable.
util/pkg/vfs/s3context.go
Outdated
| s3Client = s3.New(session, config) | ||
| sess, err := session.NewSession() | ||
| if err != nil { | ||
| return nil, err |
There was a problem hiding this comment.
Should we fmt the error here?
k8s-ci-robot
added
the
size/M
|
@chrislovecnm I think I've addressed the issues you raised on this PR, let me know if I need to take another look. |
|
/lgtm Thanks @alrs ! |
k8s-ci-robot
added
the
lgtm
From the
godoc github.com/aws/aws-sdk-go/aws/session.New:This PR replaces every use of session.New() with session.NewSession(), and additionally sets the variable name as
sessin places where it was being set tosession.