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 firehose to elastic search support #7839

Merged
merged 1 commit into from
Aug 8, 2016

Conversation

iceycake
Copy link
Contributor

Attempting to add firehose stream to elastic search support as stated in #6680

@iceycake
Copy link
Contributor Author

Please review the elasticsearch_configuration schema before I actually code the rest.

@stack72
Copy link
Contributor

stack72 commented Jul 28, 2016

Hi @iceycake

The schema looks solid - I think the only thing that seems to be missing is retry_options

The rest looks ready to get started with the CRUD

P.

@iceycake
Copy link
Contributor Author

@stack72 Interesting, the retry_options param is also missing from the redshift configuration (#7375). I wonder if there is any reason behind it?

@iceycake iceycake force-pushed the firehose-elasticsearch branch 9 times, most recently from 8172d02 to 9d6f88c Compare August 2, 2016 22:56
@iceycake iceycake changed the title [WIP] Adding firehose to elastic search support Adding firehose to elastic search support Aug 3, 2016
@iceycake iceycake changed the title Adding firehose to elastic search support [WIP] Adding firehose to elastic search support Aug 4, 2016
@iceycake iceycake force-pushed the firehose-elasticsearch branch 5 times, most recently from 6ee2d94 to 12675e3 Compare August 5, 2016 22:08
@iceycake iceycake changed the title [WIP] Adding firehose to elastic search support Adding firehose to elastic search support Aug 5, 2016
@iceycake
Copy link
Contributor Author

iceycake commented Aug 5, 2016

@stack72 The acceptance test took a loooooooong to run but I think I got it.

USHOLACH01-ML:terraform andy.chan$ make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSKinesisFirehoseDeliveryStream_ElasticsearchConfigUpdates'
==> Checking that code complies with gofmt requirements...
/Users/andy.chan/Projects/go/bin/stringer
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/08/05 11:38:23 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSKinesisFirehoseDeliveryStream_ElasticsearchConfigUpdates -timeout 120m
=== RUN   TestAccAWSKinesisFirehoseDeliveryStream_ElasticsearchConfigUpdates
--- PASS: TestAccAWSKinesisFirehoseDeliveryStream_ElasticsearchConfigUpdates (1684.12s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    3009.142s

Please review the PR. Thanks.

@iceycake iceycake force-pushed the firehose-elasticsearch branch 2 times, most recently from ee012c5 to b2c565c Compare August 5, 2016 22:54
@iceycake
Copy link
Contributor Author

iceycake commented Aug 6, 2016

Tune the timeout settings and it's way better now.

USHOLACH01-ML:terraform andy.chan$ make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSKinesisFirehoseDeliveryStream_ElasticsearchConfigUpdates'
==> Checking that code complies with gofmt requirements...
/Users/andy.chan/Projects/go/bin/stringer
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/08/05 15:55:01 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSKinesisFirehoseDeliveryStream_ElasticsearchConfigUpdates -timeout 120m
=== RUN   TestAccAWSKinesisFirehoseDeliveryStream_ElasticsearchConfigUpdates
--- PASS: TestAccAWSKinesisFirehoseDeliveryStream_ElasticsearchConfigUpdates (1712.07s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    1712.092s

Add firehose elasticsearch configuration documentation

Adding CRUD for elastic search as firehose destination

Updated the firehose stream documentation to add elastic search as destination example.

Adding testing for es as firehose destination

Update the test case for es
@stack72
Copy link
Contributor

stack72 commented Aug 8, 2016

Hi @iceycake

Thanks for the work here :)

This LGTM! Functionality works as expected

% make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSKinesisFirehoseDeliveryStream_'
==> Checking that code complies with gofmt requirements...
/Users/stacko/Code/go/bin/stringer
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/08/08 11:18:43 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSKinesisFirehoseDeliveryStream_ -timeout 120m
=== RUN   TestAccAWSKinesisFirehoseDeliveryStream_s3basic
--- PASS: TestAccAWSKinesisFirehoseDeliveryStream_s3basic (185.49s)
=== RUN   TestAccAWSKinesisFirehoseDeliveryStream_s3ConfigUpdates
--- PASS: TestAccAWSKinesisFirehoseDeliveryStream_s3ConfigUpdates (326.47s)
=== RUN   TestAccAWSKinesisFirehoseDeliveryStream_RedshiftConfigUpdates
--- PASS: TestAccAWSKinesisFirehoseDeliveryStream_RedshiftConfigUpdates (820.64s)
=== RUN   TestAccAWSKinesisFirehoseDeliveryStream_ElasticsearchConfigUpdates
--- PASS: TestAccAWSKinesisFirehoseDeliveryStream_ElasticsearchConfigUpdates (2157.09s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    3489.719s

Merging now!

Paul

@stack72 stack72 merged commit 5ac8ae1 into hashicorp:master Aug 8, 2016
@ghost
Copy link

ghost commented Apr 23, 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 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants