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.

Sign up for GitHub

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

Add support Ruby 3.3.0+ #53

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Add support Ruby 3.3.0+ #53

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a74440b
Allow use td-client v2.0+
akito19 Feb 2, 2024
a7a81bf
Drop Ruby EOL versions and add newer version
akito19 Feb 9, 2024
8e8a971
Add `mutex_m` gem for Ruby 3.3+
akito19 Feb 9, 2024
50ba787
Require td-client-ruby 1.0.8+
akito19 May 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,11 @@ jobs:
os:
- ubuntu-latest
ruby:
- 2.3.1
- 2.4.1
- 2.5.9
- 2.6.9
- 2.7.5
- 3.0.3
- 3.1.0
- 3.1.4
- 3.2.3
- 3.3.0
- head
# TODO: Add jruby
include:
Expand Down
1 change: 1 addition & 0 deletions lib/td/logger/td_logger.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ def initialize(tag_prefix, options={})
require 'cgi'
require 'logger'
require 'td-client'
require 'mutex_m'

@logger = ::Logger.new(STDERR)
if debug
Expand Down
4 changes: 2 additions & 2 deletions spec/spec_helper.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@
unless ENV['APPVEYOR']
require 'coveralls'

SimpleCov.formatter = SimpleCov::Formatter::MultiFormatter[
SimpleCov.formatter = SimpleCov::Formatter::MultiFormatter.new([
Copy link
Member Author

@akito19 akito19 Feb 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow up warnings,

/home/runner/work/td-logger-ruby/td-logger-ruby/spec/spec_helper.rb:9:in `<top (required)>': [DEPRECATION] ::[] is deprecated. Use ::new instead.

SimpleCov::Formatter::HTMLFormatter,
Coveralls::SimpleCov::Formatter
]
])
end
SimpleCov.start("test_frameworks")
end
Expand Down
1 change: 1 addition & 0 deletions spec/td/logger/agent/rack_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
require 'spec_helper'
require "rack/mock"
require 'rack/builder'
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added this line because my local test failed due to uninitialized constant.

Failures:

  1) TreasureData::Logger::Agent::Rack::Hook.before
     Failure/Error:
       @app ||= Rack::Builder.new {
         use TreasureData::Logger::Agent::Rack::Hook
         run lambda {|env| [200, {}, ["body"]]}
       }.to_app

     NameError:
       uninitialized constant Rack::Builder
     # ./spec/td/logger/agent/rack_spec.rb:7:in `app'
     # ./spec/td/logger/agent/rack_spec.rb:15:in `block (2 levels) in <top (required)>'
     # ./spec/td/logger/agent/rack_spec.rb:22:in `block (3 levels) in <top (required)>'

  2) TreasureData::Logger::Agent::Rack::Hook.after
     Failure/Error:
       @app ||= Rack::Builder.new {
         use TreasureData::Logger::Agent::Rack::Hook
         run lambda {|env| [200, {}, ["body"]]}
       }.to_app

     NameError:
       uninitialized constant Rack::Builder
     # ./spec/td/logger/agent/rack_spec.rb:7:in `app'
     # ./spec/td/logger/agent/rack_spec.rb:15:in `block (2 levels) in <top (required)>'
     # ./spec/td/logger/agent/rack_spec.rb:33:in `block (3 levels) in <top (required)>'

Finished in 0.30078 seconds (files took 1.03 seconds to load)
147 examples, 2 failures, 5 pending

require 'td/logger/agent/rack'

describe TreasureData::Logger::Agent::Rack::Hook do
Expand Down
1 change: 0 additions & 1 deletion spec/td/logger/event_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

require 'spec_helper'

describe TreasureData::Logger::Event do
Expand Down
3 changes: 2 additions & 1 deletion td-logger.gemspec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,9 @@ EOF
gem.require_paths = ['lib']

gem.add_dependency "msgpack", ">= 0.5.6", "< 2.0"
gem.add_dependency "td-client", ">= 0.8.66", "< 2.0"
gem.add_dependency "td-client", ">= 1.0.8", "< 3.0"
gem.add_dependency "fluent-logger", ">= 0.5.0", "< 2.0"
gem.add_dependency "mutex_m", ">= 0.2.0", "< 1.0"
Copy link
Member Author

@akito19 akito19 Feb 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mutex_m was independent as a Ruby gem from Ruby.
https://www.ruby-lang.org/en/news/2023/12/11/ruby-3-3-0-rc1-released/

If users have used this gem with Ruby 2.4 or lower, they failed td-logger process because this gem supports Ruby 2.5+. But I believe this is no problem since Ruby 2.6 or lower reached EOL.
https://github.com/ruby/mutex_m/blob/2a0c5af457a85b91ca39122511658661479f149e/.github/workflows/test.yml#L10

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry, but I don't understand why this added line is needed? If we're targeting ruby 2.7..5 and up here, IIUC, user should be able to use their own mutex_m without issue, no?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tung-vu-td Sorry for my late response. mutex_m will be apart from Ruby as a standard library.
If I removed the line, test case (Ruby 3.4+) got an error.
https://github.com/treasure-data/td-logger-ruby/actions/runs/9186667898/job/25262771815

Since this library uses mutex process, I believe this gem should be needed.

td-logger-ruby/lib/td/logger/td_logger.rb

Line 164 in d90b9d1

@mutex.unlock

gem.add_development_dependency 'rake', '>= 0.9.2'
gem.add_development_dependency 'rspec', '~> 3.0'
gem.add_development_dependency 'coveralls'
Expand Down
Loading