-
Notifications
You must be signed in to change notification settings - Fork 120
CUMULUS-4788: Move replication for different tables into separate modules #4403
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
Changes from all commits
fdf41fb
798a5e8
7fd8bfa
e9b172d
ecfb2c8
87337a6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| output "iceberg_replication_cluster_arn" { | ||
| description = "The ARN of the ECS cluster created by this module" | ||
| value = module.rds_iceberg_replication.iceberg_replication_cluster_arn | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,9 +20,9 @@ variable "region" { | |
| default = "us-east-1" | ||
| } | ||
|
|
||
| variable "subnets" { | ||
| description = "Subnets for database cluster. Requires at least 2 across multiple AZs" | ||
| type = list(string) | ||
| variable "subnet" { | ||
| description = "Subnet for Fargate tasks" | ||
| type = string | ||
| } | ||
|
|
||
| variable "tags" { | ||
|
|
@@ -76,10 +76,36 @@ variable "connect_image" { | |
| type = string | ||
| } | ||
|
|
||
| variable "bootstrap_image" { | ||
| description = "Image used to start the bootstrap container. See https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_ContainerDefinition.html#ECS-Type-ContainerDefinition-image" | ||
| type = string | ||
| } | ||
|
|
||
| variable "data_persistence_remote_state_config" { | ||
| type = object({ bucket = string, key = string, region = string }) | ||
| } | ||
|
|
||
| variable "rds_cluster_remote_state_config" { | ||
| type = object({ bucket = string, key = string, region = string }) | ||
| } | ||
|
|
||
| variable "iceberg_s3_bucket" { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like we aren't creating these buckets in Terraform - is the plan to create them via the script that sets up the Iceberg tables?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought that was what you said, but we can create them in the terraform if you want. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok I think a terraform variable here and created by the loading script is probably the most flexible - we can always change our minds later. |
||
| description = "S3 bucket where iceberg tables are stored" | ||
| type = string | ||
| } | ||
|
|
||
| variable "iceberg_namespace" { | ||
| description = "iceberg namespace (same as glue database)" | ||
| type = string | ||
| } | ||
|
|
||
| variable "pg_db" { | ||
| description = "postgres database" | ||
| type = string | ||
| } | ||
|
|
||
| variable "pg_schema" { | ||
| description = "The name of the schema in the postgres database that contains the tables" | ||
| type = string | ||
| default = "public" | ||
| } | ||
This file was deleted.
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.
I think the db name should be coming from an output of the RDS module.
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.
I don't think it can? The terraform is not setting up the dB, it it? That's some script running migrations, I tihnk
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.
In that case we should make it a variable to be passed in instead of hardcoding to postgres.
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.
Done