Skip to content

Commit

Permalink
Add Rubocop and get Rubocop passing
Browse files Browse the repository at this point in the history
This commit adds Rubocop and makes a number of changes to address open lints.

In addition it:

1. Sets the minimum Ruby version to 2.6, removing 2.5 and jruby-9.2 from the CI matrix.
2. Adds Ruby 3.1, jruby-9.3, ruby-head, and jruby-head to the CI matrix
3. Adds missing specs for the client method on Dalli::Elasticache
4. Adds a linting GitHub action
  • Loading branch information
petergoldstein committed Jan 20, 2022
1 parent a69a747 commit 958357d
Show file tree
Hide file tree
Showing 18 changed files with 309 additions and 146 deletions.
17 changes: 17 additions & 0 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
name: Lint

on: [push, pull_request]

jobs:
lint:
runs-on: ubuntu-latest
name: Lint - Ruby 2.6
steps:
- uses: actions/checkout@v2
- name: Set up Ruby 2.6
uses: ruby/setup-ruby@v1
with:
ruby-version: 2.6
bundler-cache: true # 'bundle install' and cache
- name: Run Rubocop
run: bundle exec rubocop
5 changes: 3 additions & 2 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@ on: [push, pull_request]
jobs:
test:
runs-on: ubuntu-latest

strategy:
fail-fast: false
matrix:
ruby-version: [2.5, 2.6, 2.7, '3.0', jruby-9.2, jruby-9.3]
ruby-version: [2.6, 2.7, '3.0', 3.1, jruby-9.3, ruby-head, jruby-head]

name: Specs - Ruby ${{ matrix.ruby-version }}
steps:
- uses: actions/checkout@v2
- name: Set up Ruby ${{ matrix.ruby-version }}
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
*.rbc
.bundle
.config
.ruby-version
coverage
InstalledFiles
lib/bundler/man
Expand Down
30 changes: 30 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
require:
- rubocop-performance
- rubocop-rake
- rubocop-rspec

AllCops:
NewCops: enable
TargetRubyVersion: 2.6

Metrics/BlockLength:
Max: 50
Exclude:
- 'spec/**/*'

Naming/FileName:
Exclude:
- 'lib/dalli-elasticache.rb'

Style/Documentation:
Exclude:
- 'spec/**/*'

RSpec/ExampleLength:
Enabled: false

RSpec/MultipleExpectations:
Enabled: false

RSpec/MultipleMemoizedHelpers:
Enabled: false
2 changes: 2 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
# frozen_string_literal: true

source 'https://rubygems.org'
gemspec
12 changes: 7 additions & 5 deletions Rakefile
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
require "rubygems"
require "bundler/setup"
require "bundler/gem_tasks"
require "rspec/core/rake_task"
# frozen_string_literal: true

require 'rubygems'
require 'bundler/setup'
require 'bundler/gem_tasks'
require 'rspec/core/rake_task'

RSpec::Core::RakeTask.new(:test)

task :default => :test
task default: :test
41 changes: 22 additions & 19 deletions dalli-elasticache.gemspec
Original file line number Diff line number Diff line change
@@ -1,34 +1,37 @@
# -*- encoding: utf-8 -*-
lib = File.expand_path('../lib/', __FILE__)
$:.unshift lib unless $:.include?(lib)
# frozen_string_literal: true

require 'dalli/elasticache/version'
require_relative './lib/dalli/elasticache/version'

Gem::Specification.new do |s|
s.name = 'dalli-elasticache'
s.version = Dalli::ElastiCache::VERSION
s.licenses = ['MIT']

s.summary = "Configure Dalli clients with ElastiCache's AutoDiscovery"
s.description = <<-EOS
s.description = <<-DESC
This gem provides an interface for fetching cluster information from an AWS
ElastiCache AutoDiscovery server and configuring a Dalli client to connect
to all nodes in the cache cluster.
EOS
s.authors = ["Aaron Suggs", "Zach Millman"]
s.email = ["[email protected]", "[email protected]"]
DESC

s.authors = ['Aaron Suggs', 'Zach Millman', 'Peter M. Goldstein']
s.email = ['[email protected]', '[email protected]', '[email protected]']
s.homepage = 'http://github.com/ktheory/dalli-elasticache'
s.files = Dir.glob("{bin,lib}/**/*") + %w(README.md Rakefile)
s.rdoc_options = ["--charset=UTF-8"]
s.require_paths = ["lib"]
s.test_files = Dir.glob("{test,spec}/**/*")
s.required_ruby_version = '>= 1.9.2' # Maybe less?

s.files = Dir.glob('{bin,lib}/**/*') + %w[README.md Rakefile]
s.rdoc_options = ['--charset=UTF-8']
s.require_paths = ['lib']
s.test_files = Dir.glob('{test,spec}/**/*')

s.required_ruby_version = '>= 2.6.0'
s.required_rubygems_version = '>= 1.3.5'

s.add_development_dependency 'rake'
s.add_development_dependency 'rspec'
s.add_dependency 'dalli', ">= 1.0.0" # ??
s.add_development_dependency 'rubocop'
s.add_development_dependency 'rubocop-performance'
s.add_development_dependency 'rubocop-rake'
s.add_development_dependency 'rubocop-rspec'
s.add_dependency 'dalli', '>= 2.0.0'
s.metadata['rubygems_mfa_required'] = 'true'
end
2 changes: 2 additions & 0 deletions lib/dalli-elasticache.rb
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
# frozen_string_literal: true

# Support default bundler require path
require 'dalli/elasticache'
40 changes: 30 additions & 10 deletions lib/dalli/elasticache.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

require 'dalli'
require 'socket'
require 'dalli/elasticache/version'
Expand All @@ -6,42 +8,60 @@
require 'dalli/elasticache/auto_discovery/stats_response'

module Dalli
##
# Dalli::Elasticache provides an interface for providing a configuration
# endpoint for a memcached cluster on ElasticCache and retrieving the
# list of addresses (IP and port) for the individual nodes of that cluster.
#
# This allows the caller to pass that server list to Dalli, which then
# distributes cached items consistently over the nodes.
##
class ElastiCache
attr_reader :endpoint, :options

def initialize(config_endpoint, options={})

##
# Creates a new Dalli::ElasticCache instance.
#
# config_endpoint - a String containing the host and (optionally) port of the
# configuration endpoint for the cluster. If not specified the port will
# default to 11211. The host must be either a DNS name or an IPv4 address. IPv6
# addresses are not handled at this time.
# dalli_options - a set of options passed to the Dalli::Client that is returned
# by the client method. Otherwise unused.
##
def initialize(config_endpoint, dalli_options = {})
@endpoint = Dalli::Elasticache::AutoDiscovery::Endpoint.new(config_endpoint)
@options = options
@options = dalli_options
end

# Dalli::Client configured to connect to the cluster's nodes
def client
Dalli::Client.new(servers, options)
end

# The number of times the cluster configuration has been changed
#
# Returns an integer
def version
endpoint.config.version
end

# The cache engine version of the cluster
def engine_version
endpoint.engine_version
end

# List of cluster server nodes with ip addresses and ports
# Always use host name instead of private elasticache IPs as internal IPs can change after a node is rebooted
def servers
endpoint.config.nodes.map{ |h| "#{h[:host]}:#{h[:port]}" }
endpoint.config.nodes.map { |h| "#{h[:host]}:#{h[:port]}" }
end

# Clear all cached data from the cluster endpoint
def refresh
config_endpoint = "#{endpoint.host}:#{endpoint.port}"
@endpoint = Dalli::Elasticache::AutoDiscovery::Endpoint.new(config_endpoint)

self
end
end
Expand Down
28 changes: 13 additions & 15 deletions lib/dalli/elasticache/auto_discovery/config_response.rb
Original file line number Diff line number Diff line change
@@ -1,48 +1,46 @@
# frozen_string_literal: true

module Dalli
module Elasticache
module AutoDiscovery

# This class wraps the raw ASCII response from an Auto Discovery endpoint
# and provides methods for extracting data from that response.
#
# http://docs.aws.amazon.com/AmazonElastiCache/latest/UserGuide/AutoDiscovery.AddingToYourClientLibrary.html

class ConfigResponse

# The raw response text
attr_reader :text

# Matches the version line of the response
VERSION_REGEX = /^(\d+)$/
VERSION_REGEX = /^(\d+)$/.freeze

# Matches strings like "my-cluster.001.cache.aws.com|10.154.182.29|11211"
NODE_REGEX = /(([-.a-zA-Z0-9]+)\|(\b\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}\b)\|(\d+))/
NODE_LIST_REGEX = /^(#{NODE_REGEX}\s*)+$/
NODE_REGEX = /(([-.a-zA-Z0-9]+)\|(\b\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}\b)\|(\d+))/.freeze
NODE_LIST_REGEX = /^(#{NODE_REGEX}\s*)+$/.freeze

def initialize(response_text)
@text = response_text.to_s
end

# The number of times the configuration has been changed
#
# Returns an integer
def version
VERSION_REGEX.match(@text)[1].to_i
end

# Node hosts, ip addresses, and ports
#
# Returns an Array of Hashes with values for :host, :ip and :port
def nodes
NODE_LIST_REGEX.match(@text).to_s.scan(NODE_REGEX).map do |match|
{
:host => match[1],
:ip => match[2],
:port => match[3].to_i
host: match[1],
ip: match[2],
port: match[3].to_i
}
end
end

end
end
end
Expand Down
Loading

0 comments on commit 958357d

Please sign in to comment.