Skip to content

Commit

Permalink
Fix deprecation warning for working_dir_path (#806)
Browse files Browse the repository at this point in the history
It would only print the warning if the config option was used in the
file source, but if the `APPSIGNAL_WORKING_DIR_PATH` environment
variable was set, it would not print the same deprecation warning. This
only makes the deprecation more visible. This is necessary before we
fully remove it in the next major version.

I've added a changeset, because it prints the warning in more scenarios
and those are scenarios we want to inform our users about before
removing the config option entirely.

Fixes #805
  • Loading branch information
tombruijn authored Jan 14, 2022
1 parent 35bd83b commit e51a8fb
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 26 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
bump: "patch"
type: "deprecate"
---

Warn about the deprecated `working_dir_path` option from all config sources. It previously only printed a warning when it was configured in the `config/appsignal.yml` file, but now also prints the warning if it's set via the Config class initialize options and environment variables. Please use the `working_directory_path` option instead.
37 changes: 11 additions & 26 deletions lib/appsignal/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -377,12 +377,9 @@ def load_from_disk
configurations = YAML.load(ERB.new(IO.read(config_file)).result, **read_options)
config_for_this_env = configurations[env]
if config_for_this_env
config_for_this_env =
config_for_this_env.each_with_object({}) do |(key, value), hash|
hash[key.to_sym] = value # convert keys to symbols
end

maintain_file_config_backwards_compatibility(config_for_this_env)
config_for_this_env.each_with_object({}) do |(key, value), hash|
hash[key.to_sym] = value # convert keys to symbols
end
else
logger.error "Not loading from config file: config for '#{env}' not found"
nil
Expand All @@ -397,26 +394,6 @@ def load_from_disk
nil
end

# Maintain backwards compatibility with config files generated by earlier
# versions of the gem
#
# Used by {#load_from_disk}. No compatibility for env variables or initial config currently.
#
# @todo This is incomplete and only considers the file source as the only
# valid config source. This should be moved to
# {maintain_backwards_compatibility}.
def maintain_file_config_backwards_compatibility(configuration)
configuration.tap do |config|
if config.include?(:working_dir_path)
deprecation_message \
"'working_dir_path' is deprecated, please use " \
"'working_directory_path' instead and specify the " \
"full path to the working directory",
logger
end
end
end

# Maintain backwards compatibility with deprecated config options.
#
# Add deprecated config options here with the behavior of setting its
Expand All @@ -442,6 +419,14 @@ def maintain_backwards_compatibility
"deprecated. Please use `send_session_data` instead.",
logger
end

if config_hash.key?(:working_dir_path) # rubocop:disable Style/GuardClause
deprecation_message \
"The `working_dir_path` option is deprecated, please use " \
"`working_directory_path` instead and specify the " \
"full path to the working directory",
logger
end
end

def load_from_environment
Expand Down
39 changes: 39 additions & 0 deletions spec/lib/appsignal/config_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -751,6 +751,45 @@
described_class.new(Dir.pwd, "production", config_options, logger)
end

describe "working_dir_path" do
let(:err_stream) { std_stream }
let(:stderr) { err_stream.read }
let(:deprecation_message) do
"The `working_dir_path` option is deprecated, please use " \
"`working_directory_path` instead and specify the " \
"full path to the working directory"
end
before do
capture_std_streams(std_stream, err_stream) { config }
end

context "when not set" do
let(:config_options) { {} }

it "sets the default working_dir_path value" do
expect(config[:working_dir_path]).to be_nil
end

it "does not print a deprecation warning" do
expect(stderr).to_not include("appsignal WARNING: #{deprecation_message}")
expect(logs).to_not include(deprecation_message)
end
end

context "when set" do
let(:config_options) { { :working_dir_path => "/tmp/appsignal2" } }

it "sets the default working_dir_path value" do
expect(config[:working_dir_path]).to eq("/tmp/appsignal2")
end

it "does not print a deprecation warning" do
expect(stderr).to include("appsignal WARNING: #{deprecation_message}")
expect(logs).to include(deprecation_message)
end
end
end

describe "skip_session_data" do
let(:err_stream) { std_stream }
let(:stderr) { err_stream.read }
Expand Down

0 comments on commit e51a8fb

Please sign in to comment.