-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
upgrade sdk go from v1 to v2 #95
Conversation
e44c1c8
to
95a5b6e
Compare
95a5b6e
to
e750fd9
Compare
dab8463
to
e7e6c48
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job
if err != nil { | ||
err := fmt.Errorf("Error searching for OMI: %s", err) | ||
state.Put("error", err) | ||
ui.Error(err.Error()) | ||
return multistep.ActionHalt | ||
} | ||
s.image = &imagesResp.Images[0] | ||
s.image = &imagesResp.GetImages()[0] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could add a test if the array is empty (in case)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What you add with the test "if the image is null" is good but check the size of the array before taking the first element.
if len(imagesResp.GetImages()) <= 0 {
err := fmt.Errorf("Error while reading the image': %s", err)
state.Put("error", err)
ui.Error(err.Error())
return multistep.ActionHalt
}
if awsErr, ok := err.(awserr.Error); ok { | ||
if awsErr.Code() == "InvalidOMIID.NotFound" || | ||
awsErr.Code() == "InvalidSnapshot.NotFound" { | ||
return false, nil | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These will not work anymore, we could plan to update them
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ll do it in a seperate PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok good
//params.Filters.SetAccountIds(oid) | ||
//params.Filters.SetAccountAliases(oali) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove comments
d51428a
to
518ce81
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job, just add this check and we will merge it 👍
02a5583
to
5f2023f
Compare
Signed-off-by: outscale_hmi <hanen.mizouni@outscale.com>
5f2023f
to
f223e4b
Compare
Signed-off-by: outscale_hmi hanen.mizouni@outscale.com