From 9c88e3fa97d01dfeddc154d8aba10f2495617d07 Mon Sep 17 00:00:00 2001 From: Kyrylo Silin Date: Fri, 2 Oct 2020 17:37:58 +0800 Subject: [PATCH] remote_settings: delete the code for config dumping/loading 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. --- lib/airbrake-ruby/remote_settings.rb | 45 +------------------ spec/remote_settings_spec.rb | 66 ---------------------------- 2 files changed, 2 insertions(+), 109 deletions(-) diff --git a/lib/airbrake-ruby/remote_settings.rb b/lib/airbrake-ruby/remote_settings.rb index a185696b..1d15d71a 100644 --- a/lib/airbrake-ruby/remote_settings.rb +++ b/lib/airbrake-ruby/remote_settings.rb @@ -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( @@ -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 @@ -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 @@ -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 diff --git a/spec/remote_settings_spec.rb b/spec/remote_settings_spec.rb index 81a82035..314da1c4 100644 --- a/spec/remote_settings_spec.rb +++ b/spec/remote_settings_spec.rb @@ -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 @@ -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 @@ -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