Skip to content

Commit

Permalink
remote_settings: delete the code for config dumping/loading
Browse files Browse the repository at this point in the history
Some of our users complain that our airbrake-ruby notifier tries to dump the
remote config that we pull from S3 to the gem directory. The OS user that is
running the code may not have write access to that directory and therefore
writing the remote config will fail.

There's no good universal location to keep persistent remote config, so it's
easier just to delete the config.
  • Loading branch information
kyrylo committed Oct 2, 2020
1 parent 209e744 commit 9c88e3f
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 109 deletions.
45 changes: 2 additions & 43 deletions lib/airbrake-ruby/remote_settings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,11 @@ module Airbrake
# config.error_notifications = data.error_notifications?
# end
#
# When {#poll} is called, it will try to load remote settings from disk, so
# that it doesn't wait on the result from the API call.
#
# When {#stop_polling} is called, the current config will be dumped to disk.
#
# @since 5.0.0
# @api private
class RemoteSettings
include Airbrake::Loggable

# @return [String] the path to the persistent config
CONFIG_DUMP_PATH = File.join(
File.expand_path(__dir__),
'../../config/config.json',
).freeze

# @return [Hash{Symbol=>String}] metadata to be attached to every GET
# request
QUERY_PARAMS = URI.encode_www_form(
Expand Down Expand Up @@ -54,18 +43,11 @@ def initialize(project_id, host, &block)
@poll = nil
end

# Polls remote config of the given project in background. Loads local config
# first (if exists).
# Polls remote config of the given project in background.
#
# @return [self]
def poll
@poll ||= Thread.new do
begin
load_config
rescue StandardError => ex
logger.error("#{LOG_LABEL} config loading failed: #{ex}")
end

@block.call(@data)

loop do
Expand All @@ -77,17 +59,11 @@ def poll
self
end

# Stops the background poller thread. Dumps current config to disk.
# Stops the background poller thread.
#
# @return [void]
def stop_polling
@poll.kill if @poll

begin
dump_config
rescue StandardError => ex
logger.error("#{LOG_LABEL} config dumping failed: #{ex}")
end
end

private
Expand Down Expand Up @@ -124,22 +100,5 @@ def build_config_uri
uri.query = QUERY_PARAMS
uri
end

def load_config
config_dir = File.dirname(CONFIG_DUMP_PATH)
Dir.mkdir(config_dir) unless File.directory?(config_dir)

return unless File.exist?(CONFIG_DUMP_PATH)

config = File.read(CONFIG_DUMP_PATH)
@data.merge!(JSON.parse(config))
end

def dump_config
config_dir = File.dirname(CONFIG_DUMP_PATH)
Dir.mkdir(config_dir) unless File.directory?(config_dir)

File.write(CONFIG_DUMP_PATH, JSON.dump(@data.to_h))
end
end
end
66 changes: 0 additions & 66 deletions spec/remote_settings_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,42 +22,19 @@
}
end

let(:config_path) { described_class::CONFIG_DUMP_PATH }
let(:config_dir) { File.dirname(config_path) }

let!(:stub) do
stub_request(:get, Regexp.new(endpoint))
.to_return(status: 200, body: body.to_json)
end

before do
# Do not create config dumps on disk.
allow(Dir).to receive(:mkdir).with(config_dir)
allow(File).to receive(:write).with(config_path, anything)
end

describe ".poll" do
describe "config loading" do
let(:settings_data) { described_class::SettingsData.new(project_id, body) }

before do
allow(File).to receive(:exist?).with(config_path).and_return(true)
allow(File).to receive(:read).with(config_path).and_return(body.to_json)

allow(described_class::SettingsData).to receive(:new).and_return(settings_data)
end

it "loads the config from disk" do
expect(File).to receive(:read).with(config_path)
expect(settings_data).to receive(:merge!).with(body).twice

remote_settings = described_class.poll(project_id, host) {}
sleep(0.2)
remote_settings.stop_polling

expect(stub).to have_been_requested.once
end

it "yields the config to the block twice" do
block = proc {}
expect(block).to receive(:call).twice
Expand All @@ -68,21 +45,6 @@

expect(stub).to have_been_requested.once
end

context "when config loading fails" do
it "logs an error" do
expect(File).to receive(:read).and_raise(StandardError)
expect(Airbrake::Loggable.instance).to receive(:error).with(
'**Airbrake: config loading failed: StandardError',
)

remote_settings = described_class.poll(project_id, host) {}
sleep(0.2)
remote_settings.stop_polling

expect(stub).to have_been_requested.once
end
end
end

context "when no errors are raised" do
Expand Down Expand Up @@ -199,32 +161,4 @@
end
end
end

describe "#stop_polling" do
it "dumps config data to disk" do
expect(Dir).to receive(:mkdir).with(config_dir)
expect(File).to receive(:write).with(config_path, body.to_json)

remote_settings = described_class.poll(project_id, host) {}
sleep(0.2)
remote_settings.stop_polling

expect(stub).to have_been_requested.once
end

context "when config dumping fails" do
it "logs an error" do
expect(File).to receive(:write).and_raise(StandardError)
expect(Airbrake::Loggable.instance).to receive(:error).with(
'**Airbrake: config dumping failed: StandardError',
)

remote_settings = described_class.poll(project_id, host) {}
sleep(0.2)
remote_settings.stop_polling

expect(stub).to have_been_requested.once
end
end
end
end

0 comments on commit 9c88e3f

Please sign in to comment.