-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
Specify kinesis stream as the source of a aws_kinesis_firehose_delivery_stream #1605
Conversation
…nesis Stream as source
Add acceptance test for aws_kinesis_firehose_delivery_stream using Kinesis stream as source
I already tested this new feature in our setup and I can confirm it works great! It will allow everyone to use a Kinesis stream directly as source of a Firehose delivery stream without requiring a Lambda function in between to connect both, making things easier and cheaper! |
|
need this feature ASAP! |
@Ninir is there anything we can do to see some progress with this PR? Thanks! |
Hi folks, Will review it this day! :) |
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.
Hey @pleschev
Just left a small nitpick to discuss, will run tests after getting your feedback on it :)
Thanks for the work, seems very good! 👍
@@ -133,6 +133,28 @@ func resourceAwsKinesisFirehoseDeliveryStream() *schema.Resource { | |||
}, | |||
}, | |||
|
|||
"source_configuration": { |
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 would keep "kinesis_source_configuration", since we can have other sources potentially in the future. Thoughts?
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.
Yeah, I agree, I'll update
Updated, ready for another review |
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.
Hey @pleschev
This looks good to me, thanks for the work! 👍
$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSKinesisFirehoseDeliveryStream'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSKinesisFirehoseDeliveryStream -timeout 120m
=== RUN TestAccAWSKinesisFirehoseDeliveryStream_s3basic
--- PASS: TestAccAWSKinesisFirehoseDeliveryStream_s3basic (161.52s)
=== RUN TestAccAWSKinesisFirehoseDeliveryStream_s3KinesisStreamSource
--- PASS: TestAccAWSKinesisFirehoseDeliveryStream_s3KinesisStreamSource (141.01s)
=== RUN TestAccAWSKinesisFirehoseDeliveryStream_s3WithCloudwatchLogging
--- PASS: TestAccAWSKinesisFirehoseDeliveryStream_s3WithCloudwatchLogging (180.35s)
=== RUN TestAccAWSKinesisFirehoseDeliveryStream_s3ConfigUpdates
--- PASS: TestAccAWSKinesisFirehoseDeliveryStream_s3ConfigUpdates (308.38s)
=== RUN TestAccAWSKinesisFirehoseDeliveryStream_ExtendedS3basic
--- PASS: TestAccAWSKinesisFirehoseDeliveryStream_ExtendedS3basic (222.77s)
=== RUN TestAccAWSKinesisFirehoseDeliveryStream_ExtendedS3InvalidProcessorType
--- PASS: TestAccAWSKinesisFirehoseDeliveryStream_ExtendedS3InvalidProcessorType (2.88s)
=== RUN TestAccAWSKinesisFirehoseDeliveryStream_ExtendedS3InvalidParameterName
--- PASS: TestAccAWSKinesisFirehoseDeliveryStream_ExtendedS3InvalidParameterName (2.77s)
=== RUN TestAccAWSKinesisFirehoseDeliveryStream_ExtendedS3Updates
--- PASS: TestAccAWSKinesisFirehoseDeliveryStream_ExtendedS3Updates (224.76s)
=== RUN TestAccAWSKinesisFirehoseDeliveryStream_RedshiftConfigUpdates
--- PASS: TestAccAWSKinesisFirehoseDeliveryStream_RedshiftConfigUpdates (947.75s)
=== RUN TestAccAWSKinesisFirehoseDeliveryStream_ElasticsearchConfigUpdates
--- PASS: TestAccAWSKinesisFirehoseDeliveryStream_ElasticsearchConfigUpdates (1255.22s)
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! |
First attempt at resolving #1601