Skip to content

Conversation

@bobbyiliev
Copy link
Collaborator

Should be merged after MaterializeInc/terraform-helm-materialize#4 and once v0.1.2 for that helm TF module is released!

Copy link
Contributor

@kay-kim kay-kim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm - left a comment about the terraform.tfvars.example

description = "Whether to install the metrics-server for the Materialize Console"
type = bool
default = true
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found the correct PR. Same apologies -- a little out of scope, but am noticing that:

  • the terraform.tfvars.example's materialize_instances have database_password and are not commented out. (As an FYI -- in the docs, I'm updating the docs now to use a separate tfvars file for the instances, terraform plan -var-file=terraform.tfvars -var-file=mz_instances.tfvars -out my-plan.tfplan`, so that we don't have to worry about commenting and uncommenting.)

  • the install_materialize_operator = false in that file (not sure if we want that to be true)

https://github.com/MaterializeInc/terraform-aws-materialize/blob/main/terraform.tfvars.example#L25C1-L43C2

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, good catch! I've removed those, I've forgotten them during the initial implementation.

@bobbyiliev bobbyiliev merged commit f89f1ba into main Jan 27, 2025
2 checks passed
@bobbyiliev bobbyiliev deleted the include-metrics-server branch January 27, 2025 20:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants