provider/aws: Add support for treat_missing_data to cloudwatch_metric_alarm #13358
Conversation
…_alarm Fixes: #13263 ``` % make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSCloudWatchMetricAlarm' ==> Checking that code complies with gofmt requirements... go generate $(go list ./... | grep -v /terraform/vendor/) 2017/04/05 08:51:06 Generated command/internal_plugin_list.go TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSCloudWatchMetricAlarm -timeout 120m === RUN TestAccAWSCloudWatchMetricAlarm_importBasic --- PASS: TestAccAWSCloudWatchMetricAlarm_importBasic (23.93s) === RUN TestAccAWSCloudWatchMetricAlarm_basic --- PASS: TestAccAWSCloudWatchMetricAlarm_basic (27.81s) === RUN TestAccAWSCloudWatchMetricAlarm_treatMissingData --- PASS: TestAccAWSCloudWatchMetricAlarm_treatMissingData (43.39s) === RUN TestAccAWSCloudWatchMetricAlarm_extendedStatistic --- PASS: TestAccAWSCloudWatchMetricAlarm_extendedStatistic (26.80s) === RUN TestAccAWSCloudWatchMetricAlarm_missingStatistic --- PASS: TestAccAWSCloudWatchMetricAlarm_missingStatistic (5.95s) PASS ok github.com/hashicorp/terraform/builtin/providers/aws 127.899s ```
| "treat_missing_data": { | ||
| Type: schema.TypeString, | ||
| Optional: true, | ||
| Computed: true, |
There was a problem hiding this comment.
I'm wondering if Default: "missing" would be more suitable here 🤔 That would make people put that field if it drifts from the default value, rather than leave it out and let anyone mess with that outside of Terraform.
There was a problem hiding this comment.
btw. new Default would probably also require state migration - not sure what's the best option here, but generally speaking I'd try to use Default over Compute (if there's an option) where possible. I think it makes the behaviour more predictable.
There was a problem hiding this comment.
but it's certainly not a blocker - just some food for thought.
There was a problem hiding this comment.
The default in web console is also "missing" so I think it make sense to keep "missing"
There was a problem hiding this comment.
let me think more about this one
This follows what the AWS API does. We had to add a state migration for
this to make sure that the user doesn't see any unexpected activity on
their Terraform plans
```
% make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAWSCloudWatchMetricAlarmMigrateState'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/04/05 14:51:32 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAWSCloudWatchMetricAlarmMigrateState -timeout 120m
=== RUN TestAWSCloudWatchMetricAlarmMigrateState
2017/04/05 14:52:13 [INFO] Found AWS CloudWatch Metric Alarm State v0; migrating to v1
2017/04/05 14:52:13 [DEBUG] Attributes before migration: map[string]string{}
2017/04/05 14:52:13 [DEBUG] Attributes after migration: map[string]string{"treat_missing_data":"missing"}
--- PASS: TestAWSCloudWatchMetricAlarmMigrateState (0.00s)
PASS
ok github.com/hashicorp/terraform/builtin/providers/aws 0.018s
```
stack72
force-pushed
the
f-aws-cloudwatch-missingdata-13263
branch
from
7fd0969
to
411144f
Compare
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
ghost
locked and limited conversation to collaborators
Fixes: #13263