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

DRY up the elasticsearch connection schema and client construction. #203

Closed
tobio opened this issue Nov 21, 2022 · 3 comments · Fixed by #220
Closed

DRY up the elasticsearch connection schema and client construction. #203

tobio opened this issue Nov 21, 2022 · 3 comments · Fixed by #220
Assignees
Labels
bug Something isn't working
Milestone

Comments

@tobio
Copy link
Member

tobio commented Nov 21, 2022

Currently the provider defines an elasticsearch block which is duplicated by the elasticsearch_connection block added to each resource.

These schemas are expected to be identical, we shouldn't be defining them twice.

Similarly, but slight more involved is the code which reads these blocks. NewApiClientFunc reads the provider level configuration whilst NewApiClient reads the resource level configuration. At a high level, their behaviour is the same, we should refactor the common code. NewApiClient currently returns an error, but the consuming code promptly turns this in a Diagnostics so we should be able to just return the Diagnostics directly.

Related to #191

@tobio tobio added the bug Something isn't working label Nov 21, 2022
@webfella webfella self-assigned this Nov 22, 2022
@webfella webfella added this to the 0.5.0 milestone Nov 22, 2022
@webfella
Copy link

Added to 0.5.0, so we can get this done before adding more duplication via #100.

@Kushmaro Kushmaro removed the bug Something isn't working label Dec 5, 2022
@Kushmaro
Copy link
Contributor

Kushmaro commented Dec 5, 2022

@tobio @webfella , ahmmmm not sure if you folks mean to keep the same schema but still two connection methods? or remove the per-resource connection method completely?

@Kushmaro Kushmaro closed this as completed Dec 5, 2022
@Kushmaro Kushmaro reopened this Dec 5, 2022
@Kushmaro Kushmaro added the bug Something isn't working label Dec 5, 2022
@tobio
Copy link
Member Author

tobio commented Dec 5, 2022

Currently just sharing the schema between the two connection methods.

That said, we should remove the elasticsearch_connection block from resources. It doesn't work during import, so creates a bad initial user experience. The same functionality is already provided by the core Terraform configuration language, so we're just needlessly adding complexity to the provider.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants