Add ENV vars to manage updating period

[saneery]

#2344 #2343

Motivation

Added two ENV vars to manage updating period of average block time and market history cache

• AVERAGE_BLOCK_CACHE_PERIOD in seconds
• MARKET_HISTORY_CACHE_PERIOD in seconds

[coveralls]

Pull Request Test Coverage Report for Build 1f519d0c-9f1c-4337-ae78-fa2a8db84100

• 0 of 0 changed or added relevant lines in 0 files are covered.
• 36 unchanged lines in 9 files lost coverage.
• Overall coverage increased (+0.9%) to 80.666%

---

Files with Coverage Reduction	New Missed Lines	%
apps/indexer/lib/indexer/transform/addresses.ex	1	79.17%
apps/ethereum_jsonrpc/test/support/ethereum_jsonrpc/case/parity/http_websocket.ex	1	0.0%
apps/block_scout_web/lib/block_scout_web/controllers/chain/market_history_chart_controller.ex	2	71.43%
apps/explorer/lib/explorer/etherscan/logs.ex	2	94.59%
apps/indexer/lib/indexer/block/fetcher.ex	3	84.42%
apps/ethereum_jsonrpc/test/support/ethereum_jsonrpc/web_socket/case/parity.ex	4	0.0%
apps/ethereum_jsonrpc/lib/ethereum_jsonrpc/web_socket/web_socket_client.ex	6	76.25%
apps/explorer/lib/explorer/chain/transaction.ex	7	84.0%
apps/block_scout_web/lib/block_scout_web/controllers/api/rpc/eth_controller.ex	10	88.51%

Totals
Change from base Build 37e4be4c-dc93-4813-a37f-1eb1d4471d47:	0.9%
Covered Lines:	5186
Relevant Lines:	6429

---

💛 - Coveralls

[pasqu4le]

I think we should avoid raising and rescuing from errors in the configuration file

[pasqu4le]

It seems excessive to me to have a try block here. I think it's better to use Integer.parse to avoid raising errors.

Considering also that this variable is used only in one place (passed to a timer.seconds) you could change the block to be:

average_block_period =
  case Integer.parse(System.get_env("ADDRESS_WITH_BALANCES_UPDATE_INTERVAL", "")) do
    {secs, ""} -> :timer.seconds(secs)
    _ -> :timer.minutes(30)
  end

This would take care of the cases where the env var is set, not set and poorly set just as well.

[saneery]

What's bad with try?

[pasqu4le]

There is nothing inherently bad with try\rescue (maybe only a slightly worst performance, but it would not matter here), but the reason it exists is to recover from an unexpected error.

In this case you used rescue not for something unexpected, but to use a default value after some condition are met (env var not set or wrongly set).

Also generally speaking using rescue as part of the logic will "hide" that a truly unexpected error was raised (and where) so I'd say it is best practice to avoid doing so (even tho it's hardly the case here because not much is going on)

[pasqu4le]

same point as above

[saneery]

@pasqu4le updated

[pasqu4le]

@saneery I see you have updated the parsing, but now :timer.seconds here (and for market_history_cache_ttl as well) should be removed, because it is already handled on the variable creation.

[saneery]

Oh, I forgot about them 😬

[vbaranov]

@saneery
And still I would suggest renaming in order of naming generalization of cache parameters:
AVERAGE_BLOCK_PERIOD -> AVERAGE_BLOCK_CACHE_PERIOD
MARKET_HISTORY_CACHE_TTL -> MARKET_HISTORY_CACHE_PERIOD

[saneery]

And still I would suggest renaming in order of naming generalization of cache parameters:

yes, I think you're right