search

toptal / chewy

High-level Elasticsearch Ruby framework based on the official elasticsearch-ruby client
MIT License
1.88k stars 364 forks source link

Relax allowed `elasticsearch` dependency version #933

Closed mjankowski closed 4 months ago

mjankowski commented 5 months ago

Saw the comment here - https://github.com/toptal/chewy/issues/847#issuecomment-1824503608 - which notes that newer 7.x versions of elasticsearch gem allow either Faraday 1.x or 2.x.

This change might be a good stopgap to include in a Chewy 7.x release while work on ES8 support (in Chewy 8) continues on that PR.

mjankowski commented 4 months ago

Any feedback on this direction?

konalegi commented 4 months ago

I'll take a loon on Monday

konalegi commented 4 months ago

yeah, let's try

konalegi commented 4 months ago

@mjankowski can you please rebase pr?

mjankowski commented 4 months ago

Rebased.

konalegi commented 4 months ago

I'm afraid we cannot simply bump the ES version. With elastic search 7.17 all specs are failing

mjankowski commented 4 months ago

Hmm, yeah. I had issues getting the full suite to run locally for master branch (ES issue, I think), so had not been able to correctly compare before. I sorted that out and now see passing master but same failures as CI with this PR branch. Looks like something changed with how you access the logger/tracer between versions 7.13.x and 7.14.x.

I just pushed another change which:

This passes for me locally.

konalegi commented 4 months ago

Specs are passing, let me try it in our internal projects, if specs there will pass as well, I'll release it. Thanks for the change btw

konalegi commented 4 months ago

yep, works fine, please update changelog as here https://github.com/toptal/chewy/pull/925/files#diff-06572a96a58dc510037d5efa622f9bec8519bc1beab13c9f251e97e657a9d4edR7

mjankowski commented 4 months ago

Pushed changelog commit.

konalegi commented 4 months ago

Thank you! Merging!