-
Notifications
You must be signed in to change notification settings - Fork 87
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
Support for transforms #284
Conversation
💚 CLA has been signed |
9db5648
to
df3201b
Compare
Both approaches will work, and there doesn't seem to be an idiomatic approach here (some providers have a property on the resource, some have a separate resource). If you're happy with the property then we can start with that. |
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.
This is awesome! Some comments inline.
Also can we add a changelog entry here in this PR |
df3201b
to
603e586
Compare
…geo/terraform-provider-elasticstack into 49-support-for-transforms
@tobio , thank you for the feedback, really appreciated. As you can tell, I'm still learning Go :) . I made a couple of pushes to fix most (if not all) of the issues. Not really sure about the "Resolve conversation" policy (who should be pushing those buttons), so I left that untouched. |
It's totally ok for you to hit those buttons. Personally I find that easier since it's clear that you've made changes to address that. |
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.
Excellent work! There's a lot of code in this contribution, thanks for taking the time to improve the provider :)
This PR is intended to add support for transforms (https://www.elastic.co/guide/en/elasticsearch/reference/current/transforms.html).
This is my first attempt to contribute to an open-source project; being new to this, I apologize in advance if I inadvertently break some rules/guidelines. Also, full disclaimer, Go is not my main programming language and I haven't done any previous coding for Terraform providers.
Feedback is welcome (obviously). While in its current state the code is functional (at least during the minimal tests I've performed), I'd like to point out a couple of design choices (in the current code) that may not be optimal:
pivot
andlatest
props are handled as raw json content;enabled
property. A different (better?) choice might have been a dedicated resource (such aselasticstack_elasticsearch_transform_state
), because start/stop is a different API flow (not part of the usual CRUD).Open points:
import
functionalityResolves #49