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

Feat: add consul datacenters #293

Merged
merged 2 commits into from
Jan 31, 2022

Conversation

andyrepton
Copy link

This Pull request adds the consul_datacenters data source to the terraform-provider for consul. This is my first Pull Request in Golang, so constructive criticism is welcome! I've added Docs and a test as well.

Thanks!

@hashicorp-cla
Copy link

hashicorp-cla commented Jan 31, 2022

CLA assistant check
All committers have signed the CLA.

@andyrepton
Copy link
Author

The force push was me fixing the email address used in the commit

@remilapeyre
Copy link
Collaborator

Hi @Seth-Karlo, welcome to Go development and in this repo! Thanks for taking the time to submit this PR, there is nothing to criticise really, I only updated dataSourceConsulDatacentersRead() to use stateWriter that removes some of the boilerplate to write the state (not that it makes much difference here but most of the other resources will use it in the end so I did it for the sake of eventual consistency), and added an additional test.

I had no idea that the API returned this information ready to consume so I had pushed back working on #290 but /v1/catalog/datacenters was there all along. Oops!

Regarding the failing tests in the CI it is because the rest of the tests were not updated for Consul 1.11. This is done in #292 so we can just ignore them here.

@remilapeyre remilapeyre merged commit 5baf0b9 into hashicorp:master Jan 31, 2022
@andyrepton
Copy link
Author

Thanks very much @remilapeyre !

@andyrepton andyrepton deleted the feat-add-consul-datacenters branch February 1, 2022 08:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants