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

Add new component template APIs #1458

Merged
merged 1 commit into from
Jun 16, 2021

Conversation

yann-soubeyrand
Copy link
Contributor

This is an attempt to add the new component template APIs based on the code for the new index template APIs.

Comment on lines +180 to +181
ShardsAcknowledged bool `json:"shards_acknowledged"`
Index string `json:"index,omitempty"`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I copied this from indices_delete_index_template.go but I didn't know where to check if these fields can appear in a response (they don't when I do a simple request to the API).

Copy link

Choose a reason for hiding this comment

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

I guess only theacknowledged field is filled here, since we do not change something on indices here

Comment on lines +219 to +220
ShardsAcknowledged bool `json:"shards_acknowledged"`
Index string `json:"index,omitempty"`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same comment as above.

Copy link

Choose a reason for hiding this comment

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

see above

This commit implements the new Component Template APIs which come with the new
Index Template APIs as described in
https://www.elastic.co/guide/en/elasticsearch/reference/7.9/index-templates.html.

Close olivere#1414

Signed-off-by: Yann Soubeyrand <[email protected]>
@ghost
Copy link

ghost commented Feb 17, 2021

@olivere This would be a really nice feature for the terraform provider, could you take a look on this PR, please :)

}
}

// Get the component template
Copy link

Choose a reason for hiding this comment

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

A Test Case for a missing template would be nice :)

Comment on lines +180 to +181
ShardsAcknowledged bool `json:"shards_acknowledged"`
Index string `json:"index,omitempty"`
Copy link

Choose a reason for hiding this comment

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

I guess only theacknowledged field is filled here, since we do not change something on indices here

Comment on lines +219 to +220
ShardsAcknowledged bool `json:"shards_acknowledged"`
Index string `json:"index,omitempty"`
Copy link

Choose a reason for hiding this comment

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

see above

@phillbaker
Copy link
Contributor

@olivere any thoughts on this?

@olivere
Copy link
Owner

olivere commented Mar 6, 2021

This is looking really good. Thank you for working on it.

Did you check that the request parameters are the ones specified by the REST API? I've seen that contributors often start by copying an existing service but then leave a lot of fields that are actually not allowed for the new service, or miss some that are. It's pretty hard to figure out because some fields are allowed on every service, and are still not specified in the REST API spec for that specific service.

It's also worth checking the Java code as the ultimate source of truth, when in doubt. E.g. those toXContent methods describe how ES serializes the JSON—documentation, while really good, is not enough in many cases.

@yann-soubeyrand
Copy link
Contributor Author

Hi @olivere, sorry for the delayed answer… I think I did some mistakes on the parameters, since, as you mentioned, it’s hard to find the ultimate source of truth.

I’m wondering what’s the status of this project compared to the official Go client for Elasticsearch (https://github.com/elastic/go-elasticsearch)?

@olivere
Copy link
Owner

olivere commented May 17, 2021

Hi @yann-soubeyrand! The source of trust the the REST API spec. However, they don't spec out the body of the request/response. That's when I look into the Java source. Those (from|to)XContent methods in the .*Builder classes are a safe bet.

With regards to the official Elasticsearch client, I've pinned an issue with my perspective on that. See here.

@olivere olivere added this to the 7.0.25 milestone Jun 16, 2021
@olivere olivere merged commit 37e6bfd into olivere:release-branch.v7 Jun 16, 2021
dungnx pushed a commit to dungnx/elastic that referenced this pull request Sep 16, 2021
This commit implements the Component Template APIs which come with the new
Index Template APIs as described in
https://www.elastic.co/guide/en/elasticsearch/reference/7.9/index-templates.html.

Close olivere#1414
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.

3 participants