ElasticSearch v. 8 upgrade

[ql]

Things to consider:

• HTTPS + Basic Auth are now enabled by default -- need to mention setup process in README and update CI workflows
• Index mass delete is disabled by default
• Custom mappings are disabled (https://www.elastic.co/guide/en/elasticsearch/reference/7.17/removal-of-types.html) - is it already handled? in Remove Chewy::Type, simplify DSL #783
• ES 7 compatibility mode -- do we need to support it?

[ql]

I suggest leaving this default

[konalegi]

Nice works, the things that I miss

• let's support both ES 7.x and 8.x, it will be easier to use it in our projects
• provide a documentation

If you want to keep working on it, but still deliver stuff in smaller pieces, we can create a feature branch and merge your changes there.

[konalegi]

can we try to map with regex? Or what exactly this number [1:9] mean?

[ql]

It's token location, not likely to change IMO.

https://github.com/elastic/elasticsearch/blob/9db177887820c2a210aea1c041a88c162754f034/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java#L336

    private static void throwOnConcreteValue(ObjectMapper mapper, String currentFieldName, DocumentParserContext context) {
        throw new DocumentParsingException(
            context.parser().getTokenLocation(),
            "object mapping for ["
                + mapper.fullPath()
                + "] tried to parse field ["
                + currentFieldName
                + "] as object, but found a concrete value"
        );
    }

[konalegi]

ok, let's see how it goes

[konalegi]

I see that we have very small difference between 7.x and 8.x, can we make it work with both version? For instance as a possibility, define these constants like

if es_8?
  ELASTIC_ERROR_NOT_BAD_REQUEST = Elastic::Transport::Transport::Errors::BadRequest end
else 
  ELASTIC_ERROR_NOT_BAD_REQUEST = Elasticsearch::Transport::Transport::Errors::BadRequest 
end

and use it

 def create(*args, **kwargs)
          create!(*args, **kwargs)
        rescue ELASTIC_ERROR_NOT_BAD_REQUEST

Or even make this prefix configurable

if es_8?
  ELASTIC_LIB_CONST_PREFIX = Elastic
else 
  ELASTIC_LIB_CONST_PREFIX = Elasticsearch
end

and then ELASTIC_LIB_CONST_PREFIX::Transport::Transport::Errors::BadRequest, not sure if it's going to work :D

[ql]

Again, it's not likely that we need it: #962 (comment)

Wrong button :D

[ql]

@konalegi it looks like we need to use elasticsearch-transport up to 7.17 to have compatibility mode:

here we have ELASTIC_CLIENT_APIVERSIONING flag:
https://github.com/elastic/elasticsearch-ruby/blob/7.17/elasticsearch-transport/lib/elasticsearch/transport/client.rb#L151

here we don't:
https://github.com/elastic/elastic-transport-ruby/blob/main/lib/elastic/transport/client.rb --

And official migration guide also recommends to start with 7.17 in compatibility mode, fix critical issues and then update libs to 8:
https://www.elastic.co/guide/en/elasticsearch/reference/current/rest-api-compatibility.html

[barthez]

Can we use services for that? The same way we start redis.

[konalegi]

@barthez the containers created this way is quite hard to configure (yeah, we have custom configs :D ) , given that the same containers needed for local development as well 🤷 so I find it quite convenient use docker compose based ES.

[konalegi]

Yeah, sorry, it's possible, I don't remember why we didn't use them from the start.

[amol21p]

@konalegi it looks like we need to use elasticsearch-transport up to 7.17 to have compatibility mode:

here we have ELASTIC_CLIENT_APIVERSIONING flag: https://github.com/elastic/elasticsearch-ruby/blob/7.17/elasticsearch-transport/lib/elasticsearch/transport/client.rb#L151

here we don't: https://github.com/elastic/elastic-transport-ruby/blob/main/lib/elastic/transport/client.rb --

And official migration guide also recommends to start with 7.17 in compatibility mode, fix critical issues and then update libs to 8: https://www.elastic.co/guide/en/elasticsearch/reference/current/rest-api-compatibility.html
Can one of you confirm if post this PR, this code would support both ES 7.17 and ES 8 ? Asking since I was looking into making changes for supporting ES 8 in our forked version. And it would make the migration seamless, if we can have the compatible mode

[ql]

@amol21p this code is not supposed to support ES 7.17 as per official upgrade guide. Compatibility mode is in the ES server, not in libs, so that 7.* libs can work with 8.* server, not the other way around.

So theoretically upgrade goes like this:

1. Upgrade Elasticsearch server to last 7.17 version
2. Set ELASTIC_CLIENT_APIVERSIONING=1 env flag and use Chewy 7
3. Resolve issues (guide is a bit unclear on this, I guess there are deprecation warning in Elastic logs or something like this)
4. Upgrade Elasticsearch server to 8.15.0
5. Resolve all deprecation logs warnings "compatible_api"
6. Upgrade to Chewy 8

I guess for smooth upgrade we need to make sure Chewy 7 plays nice with ES 8 with the flag ELASTIC_CLIENT_APIVERSIONING set

[amol21p]

However we had already some changes in our forked version. What would you suggest in that case ?
It would be good to move to a chewy version once with backward compatibility. Otherwise, effort would increase in the migration as we would have to resolve issues twice on ES8 once on the older chewy version and once on the newer version