feat(rds): DatabaseInstanceFromSnapshot support clusterSnapshotIdentifier
#33982
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #33982 +/- ##
=======================================
Coverage 82.39% 82.39%
=======================================
Files 120 120
Lines 6960 6960
Branches 1175 1175
=======================================
Hits 5735 5735
Misses 1120 1120
Partials 105 105
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
| * - Can't be the identifier of an Aurora DB cluster snapshot. | ||
| * | ||
| * @see https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/USER_RestoreFromMultiAZDBClusterSnapshot.html | ||
| * @default - restore from `snapshotIdentifier` |
There was a problem hiding this comment.
It seems this belongs to the default value for the snapshotIdentifier field
| * - Can't be specified when `clusterSnapshotIdentifier` is specified. | ||
| * - Must be specified when `clusterSnapshotIdentifier` isn't specified. | ||
| * | ||
| * @default - restore from `clusterSnapshotIdentifier` |
There was a problem hiding this comment.
It seems this belongs to the default value for the clusterSnapshotIdentifier field
There was a problem hiding this comment.
Here I mean if user doesn't specify snapshotIdentifier, it will use clusterSnapshotIdentifier. But agree that looks confusing.
I guess we can simply use None. How do you think ?
There was a problem hiding this comment.
Could you point out to me code snippet, how it is gonna use clusterSnapshotIdentifier, if snapshotIdentifier not specified? I do not see this logic inside the code. Maybe I am missing something.
There was a problem hiding this comment.
There is no such logic. User have to specify only one of clusterSnapshotIdentifier or snapshotIdentifier. That's why I think we can simply put @default None
There was a problem hiding this comment.
Yes, I would prefer that as well.
b3259d7 to
002817e
Compare
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
|
Thanks for your review @ozelalisen |
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 main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
|
Comments on closed issues and PRs are hard for our team to see. |
Issue # (if applicable)
Closes #33889
Reason for this change
https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/USER_RestoreFromMultiAZDBClusterSnapshot.html
Description of changes
DatabaseInstanceFromSnapshotsupportclusterSnapshotIdentifierclusterSnapshotIdentifierandsnapshotIdentifierare mutually exclusive, one must be specifiedDescription of how you validated changes
Unit + Integ
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license