-
Notifications
You must be signed in to change notification settings - Fork 158
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
build: bump golangci-lint version in build image #3195
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -55,22 +55,22 @@ func (*deleteRDSSnapshotFunc) Name() string { | |
return DeleteRDSSnapshotFuncName | ||
} | ||
|
||
func deleteRDSSnapshot(ctx context.Context, snapshotID string, profile *param.Profile, dbEngine RDSDBEngine) (map[string]interface{}, error) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be part of this PR? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This fixes an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @KastenMike I am assuming this was flagged by a linter as part of the upgrade. @hairyhum can confirm. If that's the case, there are 2 options:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Upgrading linter version is a github action, which I've already run, so it's just a matter of getting it to the |
||
func deleteRDSSnapshot(ctx context.Context, snapshotID string, profile *param.Profile, dbEngine RDSDBEngine) error { | ||
// Validate profile | ||
if err := ValidateProfile(profile); err != nil { | ||
return nil, errkit.Wrap(err, "Profile Validation failed") | ||
return errkit.Wrap(err, "Profile Validation failed") | ||
} | ||
|
||
// Get aws config from profile | ||
awsConfig, region, err := getAWSConfigFromProfile(ctx, profile) | ||
if err != nil { | ||
return nil, errkit.Wrap(err, "Failed to get AWS creds from profile") | ||
return errkit.Wrap(err, "Failed to get AWS creds from profile") | ||
} | ||
|
||
// Create rds client | ||
rdsCli, err := rds.NewClient(ctx, awsConfig, region) | ||
if err != nil { | ||
return nil, errkit.Wrap(err, "Failed to create RDS client") | ||
return errkit.Wrap(err, "Failed to create RDS client") | ||
} | ||
|
||
if !isAuroraCluster(string(dbEngine)) { | ||
|
@@ -82,16 +82,16 @@ func deleteRDSSnapshot(ctx context.Context, snapshotID string, profile *param.Pr | |
switch err.Code() { | ||
case awsrds.ErrCodeDBSnapshotNotFoundFault: | ||
log.WithContext(ctx).Print("Could not find matching RDS snapshot; might have been deleted previously", field.M{"SnapshotId": snapshotID}) | ||
return nil, nil | ||
return nil | ||
default: | ||
return nil, errkit.Wrap(err, "Failed to delete snapshot") | ||
return errkit.Wrap(err, "Failed to delete snapshot") | ||
} | ||
} | ||
} | ||
// Wait until snapshot is deleted | ||
log.WithContext(ctx).Print("Waiting for RDS snapshot to be deleted", field.M{"SnapshotID": snapshotID}) | ||
err = rdsCli.WaitUntilDBSnapshotDeleted(ctx, snapshotID) | ||
return nil, errkit.Wrap(err, "Error while waiting for snapshot to be deleted") | ||
return errkit.Wrap(err, "Error while waiting for snapshot to be deleted") | ||
} | ||
|
||
// delete Aurora DB cluster snapshot | ||
|
@@ -102,17 +102,17 @@ func deleteRDSSnapshot(ctx context.Context, snapshotID string, profile *param.Pr | |
switch err.Code() { | ||
case awsrds.ErrCodeDBClusterSnapshotNotFoundFault: | ||
log.WithContext(ctx).Print("Could not find matching Aurora DB cluster snapshot; might have been deleted previously", field.M{"SnapshotId": snapshotID}) | ||
return nil, nil | ||
return nil | ||
default: | ||
return nil, errkit.Wrap(err, "Error deleting Aurora DB cluster snapshot") | ||
return errkit.Wrap(err, "Error deleting Aurora DB cluster snapshot") | ||
} | ||
} | ||
} | ||
|
||
log.WithContext(ctx).Print("Waiting for Aurora DB cluster snapshot to be deleted") | ||
err = rdsCli.WaitUntilDBClusterDeleted(ctx, snapshotID) | ||
|
||
return nil, errkit.Wrap(err, "Error waiting for Aurora DB cluster snapshot to be deleted") | ||
return errkit.Wrap(err, "Error waiting for Aurora DB cluster snapshot to be deleted") | ||
} | ||
|
||
func (d *deleteRDSSnapshotFunc) Exec(ctx context.Context, tp param.TemplateParams, args map[string]interface{}) (map[string]interface{}, error) { | ||
|
@@ -130,7 +130,7 @@ func (d *deleteRDSSnapshotFunc) Exec(ctx context.Context, tp param.TemplateParam | |
return nil, err | ||
} | ||
|
||
return deleteRDSSnapshot(ctx, snapshotID, tp.Profile, dbEngine) | ||
return nil, deleteRDSSnapshot(ctx, snapshotID, tp.Profile, dbEngine) | ||
} | ||
|
||
func (*deleteRDSSnapshotFunc) RequiredArgs() []string { | ||
|
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.
It seems that this should use
errors.New
Question unrelated to this PR why are newlines (
\n
) being used inside the error message?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.
Or maybe errkit, but I was trying to keep the changes minimal.
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.
Changed to errkit because we already have it in this file.