Skip to content
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

Adding passthrough behavior for API Gateway integration #7801

Merged

Conversation

iceycake
Copy link
Contributor

@iceycake iceycake commented Jul 25, 2016

Should address #7475 and #7141

@stack72
Copy link
Contributor

stack72 commented Jul 25, 2016

Thanks so much for the PR here @iceycake

Just running the tests now :) Will merge if all goes green!

Paul

Credentials: credentials,
CacheNamespace: nil,
CacheKeyParameters: nil,
PassthroughBehavior: aws.String(d.Get("passthrough_behavior").(string)),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this is an optional value and has no default value, we should check to see if it is set before adding to the params

@stack72
Copy link
Contributor

stack72 commented Jul 25, 2016

Hi @iceycake

When trying to run the tests, I get the following:

% make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSAPIGatewayIntegration_'                   2 ↵
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSAPIGatewayIntegration_ -timeout 120m
=== RUN   TestAccAWSAPIGatewayIntegration_basic
--- FAIL: TestAccAWSAPIGatewayIntegration_basic (40.04s)
    testing.go:264: Step 1 error: Error applying: 1 error(s) occurred:

        * aws_api_gateway_integration.test: Error creating API Gateway Integration: BadRequestException: Invalid passthrough behavior specified. Valid options are WHEN_NO_MATCH, WHEN_NO_TEMPLATES, and NEVER
            status code: 400, request id: de9577d4-5299-11e6-b917-8f8a0247d01c
FAIL
exit status 1
FAIL    github.com/hashicorp/terraform/builtin/providers/aws    40.062s
make: *** [testacc] Error 1

Thoughts on this? I also left 1 small comment inline

Paul

@stack72 stack72 added enhancement waiting-response An issue/pull request is waiting for a response from the community provider/aws labels Jul 25, 2016
@stack72 stack72 self-assigned this Jul 25, 2016
@iceycake iceycake force-pushed the api-gateway-request-body-passthrough branch from c8f8cba to d48f312 Compare July 25, 2016 20:49
@iceycake
Copy link
Contributor Author

@stack72 Sorry about not being test carefully. Hopefully, this fixed the issue.

@iceycake iceycake force-pushed the api-gateway-request-body-passthrough branch from d48f312 to 8720248 Compare July 25, 2016 21:23
@iceycake iceycake force-pushed the api-gateway-request-body-passthrough branch from 8720248 to 3007cda Compare July 25, 2016 21:25
@radeksimko
Copy link
Member

Since the field is not ForceNew it will also need to be reflected in the Update() function, otherwise updating passthrough behaviour won't work and people are going to be forced to recreate the whole resource just to update passthrough behaviour. 😉

@stack72
Copy link
Contributor

stack72 commented Jul 26, 2016

Hi @iceycake

The changes now allow the tests to pass :)

% make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSAPIGatewayIntegration_'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSAPIGatewayIntegration_ -timeout 120m
=== RUN   TestAccAWSAPIGatewayIntegration_basic
--- PASS: TestAccAWSAPIGatewayIntegration_basic (50.23s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    50.242s

We just need to make sure that @radeksimko's comments are taken onboard, what happens when an Update to the PassthroughBehaviour is made?

P.

@radeksimko
Copy link
Member

Ignore my previous comment - I overlooked this piece of code:

 func resourceAwsApiGatewayIntegrationUpdate(d *schema.ResourceData, meta interface{}) error {
    return resourceAwsApiGatewayIntegrationCreate(d, meta)
 }

This LGTM then.

@stack72
Copy link
Contributor

stack72 commented Jul 26, 2016

ah perfect - thanks @radeksimko :)

@iceycake this LGTM! Merging now :) thanks for all the work here!

@ghost
Copy link

ghost commented Apr 24, 2020

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 ghost locked and limited conversation to collaborators Apr 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement provider/aws waiting-response An issue/pull request is waiting for a response from the community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants