Skip to content

chore(settings): deprecate and remove module based settings#942

Open
jansegre wants to merge 1 commit intomasterfrom
chore/remove-deprecated-settings-modules
Open

chore(settings): deprecate and remove module based settings#942
jansegre wants to merge 1 commit intomasterfrom
chore/remove-deprecated-settings-modules

Conversation

@jansegre
Copy link
Member

@jansegre jansegre commented Feb 8, 2024

Motivation

Using HATHOR_CONFIG_FILE has been deprecated in favor of HATHOR_CONFIG_YAML for a while now. I feel like it's time to remove it.

Acceptance Criteria

  • Log a "critical" error when HATHOR_CONFIG_FILE is used and fail loading the settings (this will fail very early on when running any command);
  • Clean up hathor/conf/get_settings.py now that it only has to load YAML settings;
  • Remove hathor/conf/mainnet.py, hathor/conf/testnet.py and hathor/conf/unittests.py;
  • Remove module settings tests from tests/others/test_hathor_settings.py;
  • Fix tests/resources/p2p/test_status.py (it was still using module settings to get genesis hashes).

Checklist

  • If you are requesting a merge into master, confirm this code is production-ready and can be included in future releases as soon as it gets merged

@jansegre jansegre force-pushed the chore/remove-deprecated-settings-modules branch from 69d0def to dbc7a21 Compare February 8, 2024 13:44
@codecov
Copy link

codecov bot commented Feb 8, 2024

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (8b13de7) 85.32% compared to head (dbc7a21) 85.43%.

Files Patch % Lines
hathor/conf/get_settings.py 40.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #942      +/-   ##
==========================================
+ Coverage   85.32%   85.43%   +0.11%     
==========================================
  Files         290      287       -3     
  Lines       22477    22450      -27     
  Branches     3380     3379       -1     
==========================================
+ Hits        19179    19181       +2     
+ Misses       2626     2606      -20     
+ Partials      672      663       -9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Returns the configuration named tuple.

Tries to get the configuration from a python module in the 'HATHOR_CONFIG_FILE' env var, which will be deprecated.
If not found, tries to get it from a yaml filepath in the 'HATHOR_YAML_CONFIG', which will be the new standard.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
If not found, tries to get it from a yaml filepath in the 'HATHOR_YAML_CONFIG', which will be the new standard.
Gets the configuration from a yaml filepath in the 'HATHOR_YAML_CONFIG'.
Before it, tries to get from a python module, which was deprecated, and throws a RuntimeError in case it's found.

@msbrogli msbrogli force-pushed the master branch 2 times, most recently from eb416fa to 21d7909 Compare February 12, 2026 23:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

2 participants