add new data source aws_api_gateway_rest_api #4172
Conversation
bflad
added
new-data-source
There was a problem hiding this comment.
Hi @timborrowdale 👋 Thanks for this contribution. I left some initial comments below. Can you please take a look and let me know if you have any questions?
| Type: schema.TypeString, | ||
| Required: true, | ||
| }, | ||
| "id": { |
There was a problem hiding this comment.
id does not need to be declared as a schema attribute 👍
| matchedApis = append(matchedApis, api) | ||
| } | ||
| } | ||
| return true |
There was a problem hiding this comment.
Should use return !lastPage here
| log.Printf("[DEBUG] Reading API Gateway REST APIs: %s", params) | ||
| err := conn.GetRestApisPages(params, func(page *apigateway.GetRestApisOutput, lastPage bool) bool { | ||
| for _, api := range page.Items { | ||
| if *api.Name == target { |
There was a problem hiding this comment.
To prevent a potential crash, we should wrap this with aws.StringValue(api.Name) in the unlikely scenario that its returned as nil
| return true | ||
| }) | ||
| if err != nil { | ||
| return errwrap.Wrapf("error describing API Gateway REST APIs: {{err}}", err) |
There was a problem hiding this comment.
We do not need the context of the error when returning a message back to the user here. We can use the simpler fmt.Errorf("error describing API Gateway REST APIs: %s", err) here 👍
|
|
||
| d.SetId(*match.Id) | ||
|
|
||
| if err = dataSourceAwsApiGatewayRestApiRefreshResources(d, meta); err != nil { |
There was a problem hiding this comment.
We do not need the extra function here, its contents can be included in the read function 👍
| } | ||
|
|
||
| const testAccDataSourceAwsApiGatewayRestApiConfig = ` | ||
| provider "aws" { |
There was a problem hiding this comment.
We do not need to declare the provider block in the testing framework, it defaults to us-west-2 if not provided via AWS_DEFAULT_REGION
| } | ||
|
|
||
| resource "aws_api_gateway_rest_api" "tf_test" { | ||
| name = "tf_test" |
There was a problem hiding this comment.
These should have randomized names to prevent hitting the multiple match error condition after test panics/AWS failures. Usually we achieve this by wrapping the test configuration in fmt.Sprintf() and passing a randomized name from the test function, e.g. rName := acctest.StringWithPrefix("tf_acc_test_")
bflad
added
the
waiting-response
|
@bflad Thanks for the feedback - I've had a go at addressing those issues. |
bflad
removed
the
waiting-response
|
This has been released in version 1.15.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. |
|
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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks! |
ghost
locked and limited conversation to collaborators
#4146
I've had issues trying to get the terraform-website project working to check the docs locally - I think they're ok but would be good if someone could check.