Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Relax allowed elasticsearch dependency version #933

Merged
merged 3 commits into from
May 1, 2024

Conversation

mjankowski
Copy link
Contributor

Saw the comment here - #847 (comment) - 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
Copy link
Contributor Author

Any feedback on this direction?

@konalegi
Copy link

I'll take a loon on Monday

@konalegi
Copy link

yeah, let's try

@konalegi
Copy link

@mjankowski can you please rebase pr?

@mjankowski
Copy link
Contributor Author

Rebased.

@konalegi
Copy link

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

@mjankowski
Copy link
Contributor Author

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:

  • Keeps the max version change from before
  • Adds a new min version (7.14.0)
  • Updates config code based on how access the logger and tracer values changed between 7.13.x and 7.14.x

This passes for me locally.

@konalegi
Copy link

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
Copy link

konalegi commented May 1, 2024

@mjankowski
Copy link
Contributor Author

Pushed changelog commit.

@konalegi
Copy link

konalegi commented May 1, 2024

Thank you! Merging!

@konalegi konalegi merged commit 7e65698 into toptal:master May 1, 2024
10 checks passed
@mjankowski mjankowski deleted the relax-es-versions branch May 1, 2024 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants