Remove _all field from template for ES >= 6.0
#3778
Conversation
|
Is there an issue for the ES change? Might want to link that in the commit message. |
exekias
force-pushed
the
support-es6x-templates
branch
from
323e781
to
3cdf41f
Compare
|
This is the change in ES: elastic/elasticsearch#22144 |
exekias
force-pushed
the
support-es6x-templates
branch
2 times, most recently
from
ca56ebf
to
20b28fb
Compare
filebeat/filebeat.template.json
Outdated
| @@ -1,9 +1,6 @@ | |||
| { | |||
| "mappings": { | |||
| "_default_": { | |||
| "_all": { | |||
There was a problem hiding this comment.
We should probably now call this template fielbeat.template.6x.json. Otherwise I think it is confusing that the "default" template is not for the stable version.
filebeat/filebeat.full.yml
Outdated
| #template.versions.5x.enabled: true | ||
|
|
||
| # Path to the Elasticsearch 5.x version of the template file. | ||
| #template.versions.5x.path: "${path.config}/filebeat.template-es5x.json" |
There was a problem hiding this comment.
See my comment below about the naming. For the next months until 6 is GA, loading the 5.x is going to be the expected default. I'm thinking if we should have path config now for all versions? Problem is that we add even more config options (which will be remove afterwards) :-( WDYT?
There was a problem hiding this comment.
Yep, I see the issue, had my doubts when setting it, perhaps the best thing we can do is switching to template-es6x and keep template.json as the default. Then deprecate this in 6.0 and move to the path based solution? Doing it now would break config compatibility
exekias
force-pushed
the
support-es6x-templates
branch
2 times, most recently
from
3ed1927
to
912ecf0
Compare
|
jenkins, package it please |
|
Ready for a second review :) |
| @@ -161,6 +165,8 @@ func TestOutputLoadTemplate(t *testing.T) { | |||
|
|
|||
| if strings.HasPrefix(client.Connection.version, "2.") { | |||
| templatePath = "../../libbeat.template-es2x.json" | |||
| } else if strings.HasPrefix(client.Connection.version, "5.") { | |||
|
LGTM, apart from the minor comment above. Also, please add a CHANGELOG line. |
ES 6.0 deprecates `_all`, see elastic/elasticsearch#22144. `*.template-es6x.json` has been added in case Elasticsearch 6.X is detected
exekias
force-pushed
the
support-es6x-templates
branch
from
6557075
to
7506d65
Compare
|
Nice work! |
|
@exekias Nice work. Did you manually test this one with elasticsearch master? (manually) |
|
Yes I did, worked for me :) I also checked how it was failing before my change |
|
Awesome 🎉 |
Elasticsearch 6 deprecates
_allfield, this PR adds new templates for ES6 (removing_all), and moves 5.X ones back to*.template-es5x.json, following the same logic from es2x.This change is only for 5.x branch, as master has template generation in place and I will have to tackle this from the code. Ideally I can wait for #3681 to be merged so we forget about shipping one file per ES version.