-
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
r/aws_servicecatalog_product: New resource #19122
Conversation
defaultTagsConfig := meta.(*AWSClient).DefaultTagsConfig | ||
ignoreTagsConfig := meta.(*AWSClient).IgnoreTagsConfig | ||
|
||
output, err := waiter.ProductReady(conn, d.Get("accept_language").(string), d.Id()) |
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.
Why is waiter.ProductReady
called here rather than the underlying finder
-type function?
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 guess 3 reasons. 1) There isn't a finder. Without a finder per se, the waiter is a decent finder. 2) Waiting in "read" benefits imports immediately after creation. 3) With the status/waiter/finder pattern still evolving, I'm the least sold on the merits of finders. I'm not necessarily against them but not sure they add that much until we figure out exactly how they contribute in the overall pattern. Particularly, errors need to be handled differently (mostly not found) in different contexts. The finder I like best (because it's easy to work with) so far is the one that just finds and returns errors without editorializing (changing not found to ""
or a new error). If that's all it does, then why have a finder? Adding layers of complexity isn't always bad but needs a compelling reason that I've not been convinced finders present.
5592912
to
d247d63
Compare
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 🚀.
Commercial
% make testacc TEST=./aws TESTARGS='-run=TestAccAWSServiceCatalogProduct_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSServiceCatalogProduct_ -timeout 180m
=== RUN TestAccAWSServiceCatalogProduct_basic
=== PAUSE TestAccAWSServiceCatalogProduct_basic
=== RUN TestAccAWSServiceCatalogProduct_disappears
=== PAUSE TestAccAWSServiceCatalogProduct_disappears
=== RUN TestAccAWSServiceCatalogProduct_update
=== PAUSE TestAccAWSServiceCatalogProduct_update
=== RUN TestAccAWSServiceCatalogProduct_updateTags
=== PAUSE TestAccAWSServiceCatalogProduct_updateTags
=== RUN TestAccAWSServiceCatalogProduct_physicalID
=== PAUSE TestAccAWSServiceCatalogProduct_physicalID
=== CONT TestAccAWSServiceCatalogProduct_basic
=== CONT TestAccAWSServiceCatalogProduct_updateTags
=== CONT TestAccAWSServiceCatalogProduct_physicalID
=== CONT TestAccAWSServiceCatalogProduct_update
=== CONT TestAccAWSServiceCatalogProduct_disappears
--- PASS: TestAccAWSServiceCatalogProduct_disappears (30.22s)
--- PASS: TestAccAWSServiceCatalogProduct_basic (32.40s)
--- PASS: TestAccAWSServiceCatalogProduct_updateTags (51.69s)
--- PASS: TestAccAWSServiceCatalogProduct_update (51.98s)
--- PASS: TestAccAWSServiceCatalogProduct_physicalID (73.10s)
PASS
ok github.com/terraform-providers/terraform-provider-aws/aws 76.439s
GovCloud
% make testacc TEST=./aws TESTARGS='-run=TestAccAWSServiceCatalogProduct_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSServiceCatalogProduct_ -timeout 180m
=== RUN TestAccAWSServiceCatalogProduct_basic
=== PAUSE TestAccAWSServiceCatalogProduct_basic
=== RUN TestAccAWSServiceCatalogProduct_disappears
=== PAUSE TestAccAWSServiceCatalogProduct_disappears
=== RUN TestAccAWSServiceCatalogProduct_update
=== PAUSE TestAccAWSServiceCatalogProduct_update
=== RUN TestAccAWSServiceCatalogProduct_updateTags
=== PAUSE TestAccAWSServiceCatalogProduct_updateTags
=== RUN TestAccAWSServiceCatalogProduct_physicalID
=== PAUSE TestAccAWSServiceCatalogProduct_physicalID
=== CONT TestAccAWSServiceCatalogProduct_basic
=== CONT TestAccAWSServiceCatalogProduct_updateTags
=== CONT TestAccAWSServiceCatalogProduct_physicalID
=== CONT TestAccAWSServiceCatalogProduct_update
=== CONT TestAccAWSServiceCatalogProduct_disappears
--- PASS: TestAccAWSServiceCatalogProduct_disappears (32.28s)
--- PASS: TestAccAWSServiceCatalogProduct_basic (35.28s)
--- PASS: TestAccAWSServiceCatalogProduct_update (56.65s)
--- PASS: TestAccAWSServiceCatalogProduct_updateTags (60.59s)
--- PASS: TestAccAWSServiceCatalogProduct_physicalID (75.28s)
PASS
ok github.com/terraform-providers/terraform-provider-aws/aws 78.242s
This has been released in version 3.38.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 for triage. Thanks! |
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. |
Community Note
Closes #4980
Closes #13834
Relates #18074
Relates #15369
Output from acceptance testing (GovCloud):
Output from acceptance testing (
us-west-2
):