berenddeboer / cdk-rds-sql

A CDK construct that allows creating roles and databases an on Aurora Serverless Postgresql cluster.
Apache License 2.0
23 stars 11 forks source link

feat: add support for RDS postgres instances #25

Closed emmanuelnk closed 4 months ago

emmanuelnk commented 5 months ago

This PR adds support for IDatabaseInstance as well.

Also fixes #27

berenddeboer commented 5 months ago

Thanks @emmanuelnk a lot of work went into this! But my first thought is that this should have been using a union. So there's really no need, unless you come from languages like Java, to introduce new classes.

Why can't you change the provider "cluster" property to accept an RDS instance? Your change seems much more massive than needed, although I have not spent time to investigate this.

emmanuelnk commented 5 months ago

@berenddeboer I took in your suggestion and made it such that cluster prop accepts IDatabaseInstance interface. I've tested this via my fork and it works as expected. There are now less changes and no breaking changes to the original functionality.

berenddeboer commented 5 months ago

OK, this is looking a lot more like what I expected to see!

emmanuelnk commented 4 months ago

@berenddeboer Any chance we can get this merged in?