feat(rds): database clusters from snapshots #10130
Conversation
Created the `DatabaseClusterFromSnapshot` to support creating database clusters from snapshots. I made some intentional decisions here to avoid exposing as much of the underlying "base" classes and interfaces as possible, to support future refactoring as necessary. fixes #4379
There was a problem hiding this comment.
I really like it, and I have only one suggestion.
How about we split this into multiple files? Right now, cluster.ts has 726 lines, which gets a little unwieldy.
How about having all of the base stuff like DatabaseClusterBaseProps, DatabaseClusterBase, DatabaseClusterNew, etc. in one file, and then DatabaseCluster and DatabaseClusterProps in another, and DatabaseClusterFromSnapshot and DatabaseClusterFromSnapshotProps in a third?
Of course, a different parceling is also possible. Whatever makes it easy to keep things module-private as much as possible 🙂. Let me know what you think!
Sure, seems like the same suggestion was made here and #10162. This one will be the one most impacted by a merge, so I guess we'll do it here... :) |
|
So I attempted the refactor, but ran into aws/jsii#1980. Given the refactoring would be a purely invisible, internal change, making the decision to postpone until a fix for the above has been released. |
|
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). |
See #10130 (review) for original motivation. Moves `DatabaseClusterBaseProps`, `DatabaseClusterBase`, `DatabaseClusterNew`, and `ImportedDatabaseCluster` from `cluster.ts` to `private/cluster-internal.ts`. Note that `DatabaseClusterBase` was previously publicly exported; however, that change has not yet been release, so as long as this is merged before the next release this won't be a breaking change. Originally, I thought this refactoring was blocked by aws/jsii#1980 but thanks to a clarification, I realized I just needed to mark the private classes as `@internal`.
See #10130 (review) for original motivation. Moves `DatabaseClusterBaseProps`, `DatabaseClusterBase`, `DatabaseClusterNew`, and `ImportedDatabaseCluster` from `cluster.ts` to `private/cluster-internal.ts`. Note that `DatabaseClusterBase` was previously publicly exported; however, that change has not yet been release, so as long as this is merged before the next release this won't be a breaking change. Originally, I thought this refactoring was blocked by aws/jsii#1980 but thanks to a clarification, I realized I just needed to mark the private classes as `@internal`.
Add `DatabaseClusterFromSnapshot` to support creating Aurora clusters from snapshots. Closes #10936. The logic is implemented similar to PR #10130 where the same feature was implemented for database clusters. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Add `DatabaseClusterFromSnapshot` to support creating Aurora clusters from snapshots. Closes aws#10936. The logic is implemented similar to PR aws#10130 where the same feature was implemented for database clusters. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Created the
DatabaseClusterFromSnapshotto support creating database clustersfrom snapshots.
I made some intentional decisions here to avoid exposing as much of the
underlying "base" classes and interfaces as possible, to support future
refactoring as necessary.
fixes #4379
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license