docker icon indicating copy to clipboard operation
docker copied to clipboard

fix(pelias.json): remove explicit esclient.apiVersion from projects

Open missinglink opened this issue 4 years ago • 2 comments

all the projects are going to start breaking now for the same reason as this Travis run failed.

we've got two options, either:

  • change all the esclient.apiVersion blocks to 7.6 or 7.x
  • remove them all and rely on the default pelias/config value.

I chose the latter because it simplifies the configs.

This relies on https://github.com/pelias/config/pull/126 to be merged first!

missinglink avatar Mar 26 '20 09:03 missinglink

If I had to choose, I'd update everything to 7.x.

My rationale for changing to 7.x:

  • Leaving the value in the pelias.json configs, while often redundant, also allows people to make changes to it more easily
  • Elasticsearch 8 will be coming out in the not too distant future (they already have an alpha release), so it will be nice to be able to quickly and easily swap between versions for development, as well as to upgrade the project Elasticsearch versions only as we feel confident, as we've done with previous version upgrades.
  • changes in pelias/config can come through inconsistently into the Docker images unless we take deliberate action to bump all the importer/service Docker images. A nice thing about having the apiVersion in these pelias.json files is that we can smooth over that if we need to.
  • My guess is that they are going to remove the 7.x version when Elasticsearch 8 comes out, so it could also be nice to be able to handle that quickly and easily in the docker projects.

As far as I can tell, there's no option that doesn't us require to occasionally make changes in pelias/config and/or this project to account for the way they manage the apiVersion param in the elasticsearch-js-legacy module, which is frustrating.

But it also means that ultimately we shouldn't spend too much time on thinking about this, since we will be changing things in the future regardless. So I'm happy to see this PR merged as is if you prefer.

orangejulius avatar Mar 26 '20 18:03 orangejulius

It turns out this PR was only required due to an unintentional breaking change in elasticsearch-js-legacy. I'd prefer to keep apiVersion in all the projects so that when we do upgrade Elasticsearch in the future it's clear and explicit. So I think we can close this PR.

@missinglink any thoughts?

orangejulius avatar Sep 15 '20 16:09 orangejulius