uken / fluent-plugin-elasticsearch

Apache License 2.0
890 stars 310 forks source link

Support Ruby 3.2 #998

Closed ashie closed 1 year ago

ashie commented 1 year ago

Replace obsolete File.exists? with File.exist? to support Ruby 3.2

(check all that apply)

ashie commented 1 year ago

Some tests always fail, they aren't concerned with this change:

581 tests, 959 assertions, 4 failures, 6 errors, 0 pendings, 2 omissions, 0 notifications

In addition, Coveralls always returns 400 Bad request:

[Coveralls] Submitting to https://coveralls.io/api/v1
Coveralls encountered an exception:
JSON::ParserError
unexpected token at '<html>
<head><title>400 Bad Request</title></head>
<body>
<center><h1>400 Bad Request</h1></center>
<hr><center>nginx</center>
</body>
</html>

Do you know the reason? @cosmo0920

cosmo0920 commented 1 year ago

Ruby client of Coveralls seems to be dead. The successor client is maintained here: https://github.com/tagliala/coveralls-ruby-reborn Or, completely removing is another choice for us.

ashie commented 1 year ago

Thanks.

Failed tests are same with master branch: https://github.com/uken/fluent-plugin-elasticsearch/actions/runs/4130509321/jobs/7137272846

So they aren't concerned with this PR.

cosmo0920 commented 1 year ago

Failed tests are same with master branch: https://github.com/uken/fluent-plugin-elasticsearch/actions/runs/4130509321/jobs/7137272846

So they aren't concerned with this PR.

OK. Then, we will resolve them with another PR.

ashie commented 1 year ago

Some tests always fail, they aren't concerned with this change:

581 tests, 959 assertions, 4 failures, 6 errors, 0 pendings, 2 omissions, 0 notifications

I got it, elastic-transport 8.2 seems contain incompatible changes. When I pinned elastic-transport to 8.1, they succeeded. https://github.com/ashie/fluent-plugin-elasticsearch/pull/1

But still some tests are failed on Ruby 3.1 and 3.2. I'm investigating these issues.

cosmo0920 commented 1 year ago

Yup, this PR https://github.com/elastic/elastic-transport-ruby/pull/45 introduces such incompatibility.