pelias / schema

elasticsearch schema files and tooling
MIT License
40 stars 75 forks source link

ES8 support - remove deprecated include_type_name parameter #483

Closed jnorkus closed 7 months ago

jnorkus commented 2 years ago

In ES8 the include_type_name parameter is no longer supported and causes an error when creating an index.

https://www.elastic.co/guide/en/elasticsearch/reference/7.17/removal-of-types.html

missinglink commented 2 years ago

Thanks for the PR, we aren't officially supporting 8 just yet but we'll revisit this PR when we do.

The _type deprecation has been specially handled in our codebase for some time, it used to be required, then optional and now rejected, so I'll be happy to finally see the end of it 😄

missinglink commented 2 years ago

related: https://github.com/pelias/model/pull/123, https://github.com/pelias/schema/pull/401

jnorkus commented 2 years ago

@missinglink Is there an issue/roadmap to support ES8? My colleague recently upgraded the cluster and it's not trivial to go back. Now I'm considering whether I should make necessary changes to support it. Anything else to be aware of?

missinglink commented 2 years ago

Ugh no, TBH I didn't actually realize it was released a couple weeks ago.

There are loads of companies using Pelias and only three developers actively contributing to the maintenance, so things like this take some time.

It would be helpful if you could put together a list of the important differences and breaking changes for us to review.

missinglink commented 2 years ago

Also worth mentioning that the license change makes some organizations a little uncomfortable.

I personally don't think it's a problem but it's a non-technical issue which will need to be smoothed over before we make 8 the default.

see: https://github.com/pelias/pelias/issues/926

missinglink commented 2 years ago

If omission of a _type is the only real blocker and it allows the code to interoperate between versions 7 and 8 then I'd advocate making that configurable or inferable so we don't need to make it a breaking change.

jnorkus commented 2 years ago

I managed to get index creation errors out of the way (there's more than just this parameter) but it seems that the importers are not compatible with this new index.

missinglink commented 2 years ago

The importers all use pelias/model which still sets the _type (it's configurable), so we could change that so if you set it to something like false in the config then it is omitted.

see: https://github.com/pelias/model/pull/123

I suspect this _type deprecation is the main problem with migration and suspect that us using an older deprecated version of the official elasticsearch J's client isn't helping matters.

gnanakeethan commented 1 year ago

@missinglink I am running this against OpenSearch 2.3 and removing the line in pelias/model#123 resolves the issue.

https://github.com/pelias/model/pull/123/files#diff-0e52f948868e26de3c7ee5569835fe4b9ab6eef3ce338c1e8e80a65aa9561e56L149

michaelkirk commented 7 months ago

Fixed in https://github.com/pelias/schema/pull/490