[Fix] ErrorStorage agent calls #98
Conversation
lib/boom_notifier/error_storage.ex
Outdated
| timestamp | ||
| |> default_error_storage() | ||
| |> Map.put(:accumulated_occurrences, 1), |
There was a problem hiding this comment.
Maybe we can move this to the top (in line 39) so it's more clear this is the initial value for the Agent.
Something like
initial_error_storage =
|> timestamp
|> default_error_storage()
|> Map.put(:accumulated_occurrences, 1)Or instead of calling that function default_error_storage rename it to build_error_storage and have something like this
default_error_storage =
|> timestamp
|> build_error_storage()
|> Map.put(:accumulated_occurrences, 1)
lib/boom_notifier/error_storage.ex
Outdated
| @spec reset_accumulated_errors(error_strategy, ErrorInfo.t()) :: :ok | ||
| def reset_accumulated_errors(:exponential, error_info) do | ||
| %{key: error_hash_key} = error_info | ||
| @spec reset(ErrorInfo.t()) :: __MODULE__.t() |
There was a problem hiding this comment.
Loved the rename of this function 😍
There was a problem hiding this comment.
What about something like ErrorStorage.reset_stats/2 or ErrorStorage.reset_and_get_stats/2
I'm thinking about the notifications_sender reading:
occurrences = ErrorStorage.reset_stats(error_info, notification_trigger)There was a problem hiding this comment.
makes sense to be consistent with other ErrorStorage's functions
lib/boom_notifier/error_storage.ex
Outdated
| |> Map.update!(:__max_storage_capacity__, fn current -> min(current * 2, limit) end) | ||
| end) | ||
| ) | ||
| def reset(error_info, strategy) when strategy in [:always, nil] do |
There was a problem hiding this comment.
Maybe we can remove this guard so it always fallbacks here if the strategy doesn't need to update the max_storage value
lib/boom_notifier/error_storage.ex
Outdated
| end | ||
|
|
||
| def reset_accumulated_errors(:always, error_info) do | ||
| defp reset_state(error_info, nil) do |
There was a problem hiding this comment.
NIT: defp reset_state(error_info, _limit_updater_function = nil) do
ErrorStorage.get_error_stats/1by just fetching the error entry instead of the whole state and then getting the error entry.ErrorStorage.reset_stats/1which resets an ErrorStorage entry and returns the previous occurrence.