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

GPA Improvements: 1.15 to 2.02 #79

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
20 changes: 20 additions & 0 deletions .codeclimate.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
---
engines:
duplication:
enabled: true
config:
languages:
- ruby

fixme:
enabled: true

rubocop:
enabled: true

ratings:
paths:
- "**.rb"

exclude_paths:
- lib/papertrail/okjson.rb
Copy link
Contributor

Choose a reason for hiding this comment

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

Good idea

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I usually avoid adding codeclimate.yml (less clutter), but it is needed if we want to exclude the vendored okjson (which pulls the gpa down quite a bit...)

2 changes: 1 addition & 1 deletion Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ desc "Generate #{gemspec_file}"
task :gemspec => :validate do
# read spec file and split out manifest section
spec = File.read(gemspec_file)
head, manifest, tail = spec.split(" # = MANIFEST =\n")
head, _manifest, tail = spec.split(" # = MANIFEST =\n")

# replace name version and date
replace_header(head, :name)
Expand Down
18 changes: 9 additions & 9 deletions lib/papertrail/cli.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def initialize
end

def run
if configfile = find_configfile
if configfile
configfile_options = load_configfile(configfile)
options.merge!(configfile_options)
end
Expand All @@ -38,11 +38,11 @@ def run
opts.banner = "papertrail - command-line tail and search for Papertrail log management service"
opts.version = Papertrail::VERSION

opts.on("-h", "--help", "Show usage") do |v|
opts.on("-h", "--help", "Show usage") do
puts opts
exit
end
opts.on("-f", "--follow", "Continue running and printing new events (off)") do |v|
opts.on("-f", "--follow", "Continue running and printing new events (off)") do
options[:follow] = true
end
opts.on("--min-time MIN", "Earliest time to search from") do |v|
Expand All @@ -66,18 +66,18 @@ def run
opts.on("-s", "--system SYSTEM", "System to search") do |v|
options[:system] = v
end
opts.on("-j", "--json", "Output raw JSON data (off)") do |v|
opts.on("-j", "--json", "Output raw JSON data (off)") do
options[:json] = true
end
opts.on("--color [program|system|all|off]",
[:program, :system, :all, :off],
"Attribute(s) to colorize based on (program)") do |v|
opts.on("--color [program|system|all|off]",
[:program, :system, :all, :off], "Attribute(s) to colorize based on (program)") do |v|

options[:color] = v
end
opts.on("--force-color", "Force use of ANSI color characters even on non-tty outputs (off)") do |v|
opts.on("--force-color", "Force use of ANSI color characters even on non-tty outputs (off)") do
options[:force_color] = true
end
opts.on("-V", "--version", "Display the version and exit") do |v|
opts.on("-V", "--version", "Display the version and exit") do
puts "papertrail version #{Papertrail::VERSION}"
exit
end
Expand Down
4 changes: 2 additions & 2 deletions lib/papertrail/cli_add_group.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,15 @@ def run
:token => ENV['PAPERTRAIL_API_TOKEN'],
}

if configfile = find_configfile
if configfile
configfile_options = load_configfile(configfile)
options.merge!(configfile_options)
end

OptionParser.new do |opts|
opts.banner = "papertrail-add-group"

opts.on("-h", "--help", "Show usage") do |v|
opts.on("-h", "--help", "Show usage") do
puts opts
exit
end
Expand Down
4 changes: 2 additions & 2 deletions lib/papertrail/cli_add_system.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def run
:token => ENV['PAPERTRAIL_API_TOKEN'],
}

if configfile = find_configfile
if configfile
configfile_options = load_configfile(configfile)
options.merge!(configfile_options)
end
Expand Down Expand Up @@ -59,7 +59,7 @@ def run
opts.separator ''
opts.separator "Common options:"

opts.on("-h", "--help", "Show usage") do |v|
opts.on("-h", "--help", "Show usage") do
puts opts
exit
end
Expand Down
8 changes: 6 additions & 2 deletions lib/papertrail/cli_helpers.rb
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
module Papertrail
module CliHelpers
def configfile
@configfile ||= find_configfile
end

def find_configfile
if File.exists?(path = File.expand_path('.papertrail.yml'))
if File.exist?(path = File.expand_path('.papertrail.yml'))
return path
end
if File.exists?(path = File.expand_path('~/.papertrail.yml'))
if File.exist?(path = File.expand_path('~/.papertrail.yml'))
return path
end
rescue ArgumentError
Expand Down
2 changes: 1 addition & 1 deletion lib/papertrail/cli_join_group.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ def run
:token => ENV['PAPERTRAIL_API_TOKEN'],
}

if configfile = find_configfile
if configfile
configfile_options = load_configfile(configfile)
options.merge!(configfile_options)
end
Expand Down
2 changes: 1 addition & 1 deletion lib/papertrail/cli_leave_group.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ def run
:token => ENV['PAPERTRAIL_API_TOKEN'],
}

if configfile = find_configfile
if configfile
configfile_options = load_configfile(configfile)
options.merge!(configfile_options)
end
Expand Down
2 changes: 1 addition & 1 deletion lib/papertrail/cli_remove_system.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ def run
:token => ENV['PAPERTRAIL_API_TOKEN'],
}

if configfile = find_configfile
if configfile
configfile_options = load_configfile(configfile)
options.merge!(configfile_options)
end
Expand Down
46 changes: 21 additions & 25 deletions lib/papertrail/http_client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,47 +42,27 @@ def get(path, params = {})
if params.size > 0
path = "#{path}?#{build_nested_query(params)}"
end
attempts = 0
begin

attempt_http tries: 3, wait: 5.0 do
Copy link
Contributor

Choose a reason for hiding this comment

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

I just realized this instance of #attempt_http is using the same number of tries as all the others. I think it's fine to hard code 3 tries in the method and wait to expose that option in the future if needed.

on_complete(https.get(request_uri(path), @headers))
rescue SystemCallError, Net::HTTPFatalError => e
sleep 5.0
attempts += 1
retry if (attempts < 3)
raise e
end
end

def put(path, params)
attempts = 0
begin
attempt_http do
on_complete(https.put(request_uri(path), build_nested_query(params), @headers))
rescue SystemCallError, Net::HTTPFatalError => e
attempts += 1
retry if (attempts < 3)
raise e
end
end

def post(path, params)
attempts = 0
begin
attempt_http do
on_complete(https.post(request_uri(path), build_nested_query(params), @headers))
rescue SystemCallError, Net::HTTPFatalError => e
attempts += 1
retry if (attempts < 3)
raise e
end
end

def delete(path)
attempts = 0
begin
attempt_http do
on_complete(https.delete(request_uri(path), @headers))
rescue SystemCallError, Net::HTTPFatalError => e
attempts += 1
retry if (attempts < 3)
raise e
end
end

Expand Down Expand Up @@ -156,6 +136,22 @@ def escape(s)
'%' + $&.unpack('H2' * $&.bytesize).join('%').upcase
}.tr(' ', '+')
end

def attempt_http(options={})
tries = options[:tries] || 3
wait = options[:wait]

attempts = 0
begin
yield
rescue SystemCallError, Net::HTTPFatalError => e
sleep wait if wait
attempts += 1
retry if (attempts < tries)
raise e
end
end

end

end