-
Notifications
You must be signed in to change notification settings - Fork 152
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
Update RDS postgres restore command #1118
Conversation
Add function to fetch RDS DB EngineVersion
pkg/function/restore_rds_snapshot.go
Outdated
return nil, errors.Wrapf(err, "Couldn't find DBInstance Version") | ||
} | ||
// Add Constraints | ||
constraints, err := version.NewConstraint(">= 13.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.
Lets declare version as const
Co-authored-by: Prasad Ghangal <prasad.ghangal@gmail.com>
pkg/function/restore_rds_snapshot.go
Outdated
return nil, errors.Wrapf(err, "Couldn't add constraint to DBInstance Version") | ||
} | ||
// Verify Constraints | ||
if !constraints.Check(v1) { |
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.
is it same as v1 >= RDSPostgresDBInstanceEngineVersion
?
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.
@PrasadG193 Since we have used !constraints.Check(v1)
. It will be same as v1 < RDSPostgresDBInstanceEngineVersion
which is if v1 < 13.0
. Let me reverse the constraint used in line number 201
and update the If condition to make it more readable.
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.
Looks good
pkg/function/utils.go
Outdated
} | ||
|
||
if (len(dbInstance.DBInstances) == 0) || (dbInstance.DBInstances[0].EngineVersion == nil) { | ||
return "", errors.Errorf("Received nil engine version") |
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.
should we change this to
DB instance's Engine version is nil
pkg/function/utils.go
Outdated
@@ -208,6 +208,20 @@ func findRDSEndpoint(ctx context.Context, rdsCli *rds.RDS, instanceID string) (s | |||
return *dbInstance.DBInstances[0].Endpoint.Address, nil | |||
} | |||
|
|||
// findRDSDBEngineVersion returns the database engine version | |||
func findRDSDBEngineVersion(ctx context.Context, rdsCli *rds.RDS, instanceID string) (string, error) { |
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.
func findRDSDBEngineVersion(ctx context.Context, rdsCli *rds.RDS, instanceID string) (string, error) { | |
func rdsDBEngineVersion(ctx context.Context, rdsCli *rds.RDS, instanceID string) (string, error) { |
Change Overview
This PR updates RDS Postgres restore command to replace
LOCALE
withLC_COLLATE
for the RDS Postgres DB Instance engine version less than13.0
.Pull request type
Please check the type of change your PR introduces:
Issues
Test Plan
go test -v -check.f=RDSFunctionsTest.TestPrepareCommand