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.

Sign up for GitHub

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

Jump to bottom

Add Kinesis logging support #351

Merged
merged 5 commits into from
Jan 8, 2021
Merged

Add Kinesis logging support #351

merged 5 commits into from
Jan 8, 2021

Conversation

MerlinDMC
Copy link
Contributor

Proposed Change(s)

  • Updates go-fastly to version v2.1.0 to support Kinesis.
  • Adds support for provisioning Kinesis logging endpoints.
  • Updates fastly_service_v1 and fastly_service_compute docs to reflect the change.

Relevant Link(s) / Doc(s)

Validation

export FASTLY_API_KEY="foo"; export TF_ACC=1; make testacc TESTARGS='-run=[kK]inesis'

fixes #350

@MerlinDMC MerlinDMC mentioned this pull request Jan 5, 2021
Closed
Integralist
Integralist requested changes Jan 6, 2021
View reviewed changes
Copy link
Collaborator

@Integralist Integralist left a comment

Choose a reason for hiding this comment

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

👋🏻 I ran your branch/tests locally and found they were failing so I noticed this issue in the code that when updated would cause the tests to pass.

I'll give another proper/full review tomorrow.

Thanks.

@MerlinDMC @Integralist
Update fastly/resource_fastly_service_compute.go
ed81f51 
Co-authored-by: Mark McDonnell <Integralist@users.noreply.github.com>
Integralist
Integralist reviewed Jan 7, 2021
View reviewed changes
Copy link
Collaborator

@Integralist Integralist left a comment

Choose a reason for hiding this comment

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

👋🏻 I noticed just a couple of minor corrections.

@MerlinDMC @Integralist
Apply suggestions from code review
b70bf88 
Co-authored-by: Mark McDonnell <Integralist@users.noreply.github.com>
@MerlinDMC
Copy link
Contributor Author

Copy and Paste 😞

Integralist
Integralist approved these changes Jan 7, 2021
View reviewed changes
Copy link
Collaborator

@Integralist Integralist left a comment

Choose a reason for hiding this comment

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

Awesome @MerlinDMC thank you for taking the time to put this PR together!

This looks good to me.

I'm now running the full test suite on your branch before I merge, but unfortunately it can take many hours to complete (we're working on fixing that btw).

@Integralist Integralist merged commit ad42966 into fastly:master Jan 8, 2021
@Integralist Integralist added the enhancement New feature or request label Jan 8, 2021
@MerlinDMC MerlinDMC deleted the kinesis-logging branch January 8, 2021 09:38
@MerlinDMC
Copy link
Contributor Author

👍 Thanks for taking the time to review and test it. We're looking forward to test the logging stream via Kinesis 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Integralist Integralist Integralist approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Feature Request - logging_kinesis
2 participants
@MerlinDMC @Integralist