-
Notifications
You must be signed in to change notification settings - Fork 1
feat: Supports dynamic blocks in replication_specs #37
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
Conversation
region_name = region.region_name | ||
priority = region.priority | ||
electable_specs = region.electable_nodes > 0 ? { | ||
electable_specs = region.electable_nodes == 0 ? null : { |
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.
nit: easier to read as we only need to change the first line of the object but not the last one
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.
Is it possible that any of these are nullable
? (electable_nodes, read_only_nodes, etc.)
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.
yes, read_only_nodes for sure, and in the latest PR I also allow electable_nodes to be null (e.g. a region only with read-only nodes)
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.
@lantoli, will the comparison crash if region.read_only_nodes are null?
@@ -1,10 +1,9 @@ | |||
{ | |||
"autoscaling_missing_attribute": "setting replication_specs: attribute provider_instance_size_name not found", |
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.
ordered alphabetically
Co-authored-by: Marco Suma <[email protected]>
README.md
Outdated
|
||
- [`num_shards`](https://registry.terraform.io/providers/mongodb/mongodbatlas/latest/docs/resources/cluster#num_shards-2) in `replication_specs` must be a numeric [literal expression](https://developer.hashicorp.com/nomad/docs/job-specification/hcl2/expressions#literal-expressions), e.g. `var.num_shards` is not supported. This is to allow creating a `replication_specs` element per shard in `mongodbatlas_advanced_cluster`. This limitation doesn't apply if you're using `dynamic` blocks in `regions_config` or `replication_specs`. | ||
- `dynamic` blocks are currently supported only for `tags`, `labels` and `regions_config`. See limitations for `regions_config` support in [its section](#dynamic-blocks-in-regions_config) above. **Coming soon**: support for `replication_specs`. | ||
- `dynamic` blocks are supported with some limitations for [`regions_config`](#dynamic-blocks-in-regions_config) and [`replication_specs`](#dynamic-blocks-in-replication_specs). |
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.
- `dynamic` blocks are supported with some limitations for [`regions_config`](#dynamic-blocks-in-regions_config) and [`replication_specs`](#dynamic-blocks-in-replication_specs). | |
- `dynamic` blocks are supported with small limitations documented in [`regions_config`](#dynamic-blocks-in-regions_config) and [`replication_specs`](#dynamic-blocks-in-replication_specs). |
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.
- `dynamic` blocks are supported with some limitations for [`regions_config`](#dynamic-blocks-in-regions_config) and [`replication_specs`](#dynamic-blocks-in-replication_specs). | |
- `dynamic` blocks are supported with the limitations documented in [`regions_config`](#dynamic-blocks-in-regions_config) and [`replication_specs`](#dynamic-blocks-in-replication_specs). |
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.
@EspenAlbert @oarbusi at the end I've followed a different approach and created a guide: 919fc02
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.
👏 🚀
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.
LGTM
Description
Supports dynamic blocks in replication_specs.
Link to any related issue(s): CLOUDP-303941
Type of change:
Required Checklist:
Further comments