-
Notifications
You must be signed in to change notification settings - Fork 9.2k
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
fix: Properly normalize allocation_strategy values for aws_emr_cluster #34367
fix: Properly normalize allocation_strategy values for aws_emr_cluster #34367
Conversation
Community NoteVoting for Prioritization
For Submitters
|
271c701
to
c6f7570
Compare
c6f7570
to
a49e2fc
Compare
I need some help from one of the maintainers to run the full acceptance test suite for this resource. With my environment, I am not able to scale the run to 20 parallel tests, and setting it to 5 takes forever to run. I was able to get one run to complete eventually, but there were a couple test cases that failed because of what seems to be wait time issues. AFAIK the failures were unrelated to the change. Thanks and let me know if you need anything else. |
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 🚀.
% make testacc TESTARGS='-run=TestAccEMRCluster_InstanceFleet_basic\|TestAccEMRCluster_basic\|TestAccEMRInstanceFleet_ebsBasic\|TestAccEMRInstanceFleet_full' PKG=emr ACCTEST_PARALLELISM=2
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/emr/... -v -count 1 -parallel 2 -run=TestAccEMRCluster_InstanceFleet_basic\|TestAccEMRCluster_basic\|TestAccEMRInstanceFleet_ebsBasic\|TestAccEMRInstanceFleet_full -timeout 360m
=== RUN TestAccEMRCluster_basic
=== PAUSE TestAccEMRCluster_basic
=== RUN TestAccEMRCluster_InstanceFleet_basic
=== PAUSE TestAccEMRCluster_InstanceFleet_basic
=== RUN TestAccEMRInstanceFleet_ebsBasic
=== PAUSE TestAccEMRInstanceFleet_ebsBasic
=== RUN TestAccEMRInstanceFleet_full
=== PAUSE TestAccEMRInstanceFleet_full
=== CONT TestAccEMRCluster_basic
=== CONT TestAccEMRInstanceFleet_ebsBasic
--- PASS: TestAccEMRCluster_basic (335.85s)
=== CONT TestAccEMRCluster_InstanceFleet_basic
--- PASS: TestAccEMRInstanceFleet_ebsBasic (698.66s)
=== CONT TestAccEMRInstanceFleet_full
--- PASS: TestAccEMRInstanceFleet_full (724.47s)
--- PASS: TestAccEMRCluster_InstanceFleet_basic (1589.45s)
PASS
ok github.com/hashicorp/terraform-provider-aws/internal/service/emr 1941.528s
@acwwat Thanks for the contribution 🎉 👏. |
This functionality has been released in v5.36.0 of the Terraform AWS Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you! |
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. |
Description
This PR is to fix the normalization of the
allocation_strategy
value for both thespot_specification
andon_demand_specification
configuration blocks of theaws_emr_cluster
resource. Although the AWS API takes in values with lowercase characters and dashes, the attribute value returned by the AWS is converted to uppercase and underscores thus causing inconsistencies. To work around the issue, the Terraform resource code must normalize the value to the lowercase + dash scheme when flattening these configuration blocks.Relations
Closes #34151
References
allocation_strategy
attribute consist of lowercase characters and dashes.Output from Acceptance Testing