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

Include additional kinesis-settings in dms endpoint #18750

Closed
wants to merge 8 commits into from
Closed

Include additional kinesis-settings in dms endpoint #18750

wants to merge 8 commits into from

Conversation

kduvekot-wehkamp-nl
Copy link

added the following settings:
IncludeTransactionDetails
IncludePartitionValue
PartitionIncludeSchemaTable
IncludeTableAlterOperations
IncludeControlDetails
IncludeNullAndEmpty

I am not a programmer, not am I able to check if this is correct, but I thought I would make the effort to start this pull request because in a project that I am working on we needed to have these settings.

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Relates OR Closes #0000

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccXXX'

...

added the following settings:
IncludeTransactionDetails
IncludePartitionValue
PartitionIncludeSchemaTable
IncludeTableAlterOperations
IncludeControlDetails
IncludeNullAndEmpty
@kduvekot-wehkamp-nl kduvekot-wehkamp-nl requested a review from a team as a code owner April 12, 2021 09:02
@ghost ghost added size/XS Managed by automation to categorize the size of a PR. service/databasemigrationservice labels Apr 12, 2021
@kduvekot-wehkamp-nl kduvekot-wehkamp-nl marked this pull request as draft April 12, 2021 09:02
@github-actions github-actions bot added the needs-triage Waiting for first response or review from a maintainer. label Apr 12, 2021
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Welcome @kduvekot-wehkamp-nl 👋

It looks like this is your first Pull Request submission to the Terraform AWS Provider! If you haven’t already done so please make sure you have checked out our CONTRIBUTING guide and FAQ to make sure your contribution is adhering to best practice and has all the necessary elements in place for a successful approval.

Also take a look at our FAQ which details how we prioritize Pull Requests for inclusion.

Thanks again, and welcome to the community! 😃

@ghost ghost added size/S Managed by automation to categorize the size of a PR. and removed size/XS Managed by automation to categorize the size of a PR. labels Apr 12, 2021
added the new elements to the schema of the Kinesis settings
@ghost ghost added size/M Managed by automation to categorize the size of a PR. and removed size/S Managed by automation to categorize the size of a PR. labels Apr 12, 2021
removed a "}," that should not have been removed :-)
@ghost ghost added the tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. label Apr 12, 2021
@ghost ghost added the documentation Introduces or discusses updates to documentation. label Apr 12, 2021
@kduvekot-wehkamp-nl kduvekot-wehkamp-nl marked this pull request as ready for review April 12, 2021 14:42
@kduvekot-wehkamp-nl
Copy link
Author

all checks looks to be passing .. it looks "similar" to the pull request that created the kinesis endpoint initially.
Everything was done via the GitHub webgui .. I would like to thank you for all the automated tests etc. That helped me in fixing problems that were clear from those tests.

I hope you can do a good review of this pull request .. and that it can be incorporated in the a next version. It would certainly make our platform team happy .. so they can move away from the manual updates via the CLI for this.

@kduvekot-wehkamp-nl kduvekot-wehkamp-nl changed the title [WIP] include additional kinesis-settings in dms endpoint Include additional kinesis-settings in dms endpoint Apr 12, 2021
@YakDriver
Copy link
Member

@kduvekot-wehkamp-nl Thank you for submitting this PR! It looks like a promising start. However, in order to progress, we ask that you include an acceptance test that verifies the new arguments and run the tests to make sure everything works. You'll find more here: https://github.com/hashicorp/terraform-provider-aws/blob/main/docs/contributing/running-and-writing-acceptance-tests.md

@YakDriver YakDriver removed the needs-triage Waiting for first response or review from a maintainer. label Apr 14, 2021
@YakDriver
Copy link
Member

Related #14340
Related #14620
Related #17591

@kduvekot-wehkamp-nl
Copy link
Author

Thanks for the pointers ... I will do my best to get it included.

@anGie44 anGie44 added the enhancement Requests to existing resources that expand the functionality or scope. label May 27, 2021
@ewbankkit
Copy link
Contributor

Superseded by #20904.

@ewbankkit
Copy link
Contributor

@kduvekot-wehkamp-nl Thanks for the contribution 🎉 👏.

@kduvekot-wehkamp-nl
Copy link
Author

@ewbankkit how to proceed with this one?

@ewbankkit
Copy link
Contributor

Commits incorporated into #20084.

@github-actions
Copy link

I'm going to lock this pull request 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 related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Introduces or discusses updates to documentation. enhancement Requests to existing resources that expand the functionality or scope. size/M Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants