feat(rds): validate backup retention for read replica instances#17569
feat(rds): validate backup retention for read replica instances#17569mergify[bot] merged 7 commits intoaws:masterfrom
Conversation
Automatic backups of read replica instances are only supported for MySQL and MariaDB. Closes aws#17356
|
|
skinny85
left a comment
There was a problem hiding this comment.
Looks great! Tiny comments, mainly about using a private function we have in the module (engineDescription()).
| backupRetention, | ||
| instanceType, | ||
| vpc, | ||
| })).toThrow(/Cannot set `backupRetention` for a postgres read replica/); |
There was a problem hiding this comment.
| })).toThrow(/Cannot set `backupRetention` for a postgres read replica/); | |
| expect(() =>{ | |
| new rds.DatabaseInstanceReadReplica(stack, 'Replica', { | |
| sourceDatabaseInstance: source, | |
| backupRetention, | |
| instanceType, | |
| vpc, | |
| }); | |
| }).toThrow(/Cannot set `backupRetention` for a postgres read replica/); |
(Sorry, GutHub's UI seems screwed up today, I can't select lines correctly)
There was a problem hiding this comment.
Done. A note on this { } style and why I prefer the other style for toThrow expectations: it allows to "easily" add statements inside the { } which is not a good practice when testing that a single statement should throw.
expect(() => myFun()).toThrow(); // cannot easily add statements inside the throw expectationexpect(() => {
myFun(); // can easily add statements inside (above or below)
}).toThrow();😄
There was a problem hiding this comment.
You are correct, but for me, it's more important that I see what's being tested (myFun() in this case) on a separate line 🙂.
There was a problem hiding this comment.
OK. The change has been made.
Pull request has been modified.
|
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
…17569) Automatic backups of read replica instances are only supported for MySQL and MariaDB. See https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/USER_ReadRepl.html#USER_ReadRepl.Overview.Differences Closes aws#17356 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Automatic backups of read replica instances are only supported for MySQL
and MariaDB.
See https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/USER_ReadRepl.html#USER_ReadRepl.Overview.Differences
Closes #17356
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license