New Elasticsearch client support (revisited)

[bernardocs]

Fixes #52 Revisiting PR #53 Did a rebase with master I got a little confused over the tests though... I would probably welcome some advice on these. I'll try to include it later anyway :smiley:

[bernardocs]

Also identified (here) that the new ElasticSearch client requires Node >8 ... With that said, I just removed a little destructuring that I found...

I'll try to run a linter later

[bernardocs]

The tests that worked on TravisCI fail locally Should I try to include elasticsearch and @elasticsearch on this collection to see if things run smoothly on Travis? :thinking:

[adrai]

can you remove the package-lock.json file and not yet update the version in the package.json?

[adrai]

The tests that worked on TravisCI fail locally Should I try to include elasticsearch and @elasticsearch on this collection to see if things run smoothly on Travis? 🤔

you can try to include your new @elasticsearch in that collection

[bernardocs]

ElasticSearch new package (@elastic/elasticsearch) was not in the package.json (duh) and, even so, the client has changed a little the return body... that's why the tests went red... I'll try to fix today

[bernardocs]

I noticed that the Elasticsearch store implementation tries to use TTL as a mean to invalidate sessions (correct me if I'm wrong)... but there's a problem with that: TTL feature has been deprecated since version 5.x and removed altogether on version 7.x

The tests throw an error when trying to put session mapping with _ttl saying:

Root mapping definition has unsupported parameters: [_ttl : {default=14d, enabled=true}]

Should we review the support for ElasticSearch altogether? :eyes:

Refer to (...) for more info: https://www.elastic.co/blog/ttl-documents-shield-and-found https://discuss.elastic.co/t/why-es-remove-ttl-and-how-it-is-replaced/60449/2

[adrai]

I'm not an elasticsearch expert... maybe we should ask @jackpilowsky @tony-cocco @ewjmulder ?

[jackpilowsky]

_timestamp and _ttl The _timestamp and _ttl fields were deprecated and are now removed. As a replacement for _timestamp, you should populate a regular date field with the current timestamp on application side. For _ttl, you should either use time-based indices when applicable, or cron a delete-by-query with a range query on a timestamp field.

https://www.elastic.co/guide/en/elasticsearch/reference/5.0/breaking_50_mapping_changes.html

[jackpilowsky]

I will need this regardless in the long term, but my team is not ready to move to ES 7 yet and my backlog is looking pretty long. Might take a few weeks before I can create a workaround

[ewjmulder]

@adrai I would have liked to help out but since that one fix PR in 2016 I've not worked much with elasticsearch, so I'm afraid I cannot be of any help, good luck!

[jackpilowsky]

@adrai , @bernardocs

Please check https://github.com/jackpilowsky/sessionstore/commit/d3cba7ce69a6ba4198e3ea36b2ed9127da3b44e0

I had to make significant changes to the way the library works. Some functions don't work with globbed index names (client.exists and client.get in particular).