Use settings for purging records #12552
Conversation
| @@ -4,14 +4,14 @@ module DriftState::Purging | |||
|
|
|||
| module ClassMethods | |||
| def purge_mode_and_value | |||
| value = VMDB::Config.new("vmdb").config.fetch_path(:drift_states, :history, :keep_drift_states) | |||
| value = ::Settings.fetch_path(:drift_states, :history, :keep_drift_states) | |||
There was a problem hiding this comment.
Prefer dot notation over fetch path
| mode = (value.nil? || value.number_with_method?) ? :date : :remaining | ||
| value = (value || 6.months).to_i_with_method.seconds.ago.utc if mode == :date | ||
| return mode, value | ||
| end | ||
|
|
||
| def purge_window_size | ||
| VMDB::Config.new("vmdb").config.fetch_path(:drift_states, :history, :purge_window_size) || 10000 | ||
| ::Settings.fetch_path(:drift_states, :history, :purge_window_size) || 10_000 |
There was a problem hiding this comment.
No need for defaultsin the source anymore...please move the default to settngs.yml directly.
| @@ -1,6 +1,6 @@ | |||
| module Metric::Purging | |||
| def self.purge_date(type) | |||
| value = VMDB::Config.new("vmdb").config.fetch_path(:performance, :history, type.to_sym) | |||
| value = ::Settings.fetch_path(:performance, :history, type.to_sym) | |||
There was a problem hiding this comment.
For this one you would do something like value = ::Settings.performance.history[type] (No need even for the to_sym anymore)
kbrock
force-pushed
the
purge_settings
branch
from
6ae839f
to
39f5a5b
Compare
|
Updated:
Noticed:
|
|
LGTM 👍 Just needs to go 💚 |
|
This pull request is not mergeable. Please rebase and repush. |
no longer need to test with missing config values since they are in the default settings.yml
kbrock
force-pushed
the
purge_settings
branch
from
39f5a5b
to
08f4f84
Compare
|
Checked commit kbrock@08f4f84 with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1 |
|
👍 looks good |
|
thanks @yrudman -- I changed a few things and appreciate the second look |
|
@Fryguy this is green. anything else? |
Use settings to purge records
I did need to remove 2 specs.
They were testing when settings has no value.
But the new way we handle settings, we put the default value into
settings.yaml, so that no longer made sense./cc @yrudman FYI