From 69a99a61ba75afbb636f31d8c1c406981f6f07c3 Mon Sep 17 00:00:00 2001 From: rick batka Date: Fri, 9 Feb 2018 11:29:37 -0500 Subject: [PATCH] Fix issue #930. Disallow positional arguments for CLI settings_set and fix error reporting when settings_set fails. --- CHANGELOG.md | 2 ++ docs/cli.md | 58 ++++++++++++++++----------------- lbrynet/conf.py | 20 +++++++----- lbrynet/core/Error.py | 5 +++ lbrynet/daemon/Daemon.py | 69 ++++++++++++++++++---------------------- 5 files changed, 78 insertions(+), 76 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 445f7faf2..54e777a2a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ at anytime. * ### Fixed + * Fixed improper parsing of arguments to CLI settings_set (https://github.com/lbryio/lbry/issues/930) * Fixed unnecessarily verbose exchange rate error (https://github.com/lbryio/lbry/issues/984) * Merged two separate dht test folders into one * Fixed value error due to a race condition when saving to the claim cache (https://github.com/lbryio/lbry/issues/1013) @@ -29,6 +30,7 @@ at anytime. * `get_availability`, replaced with `stream_availability` ### Changed + * Removed support for positional arguments in cli `settings_set`. Now only accepts settings changes in the form `--setting_key=value` * Check claim schema in `publish` before trying to make the claim, return better error messages * Renamed `channel_list_mine` to `channel_list` * Changed `channel_list` to include channels where the certificate info has been imported but the claim is not in the wallet diff --git a/docs/cli.md b/docs/cli.md index 9d114e695..8f0c0ff0f 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -832,28 +832,27 @@ Returns: Set daemon settings Usage: - settings_set [ | --download_directory=] - [ | --data_rate=] - [ | --download_timeout=] - [ | --peer_port=] - [ | --max_key_fee=] - [ | --disable_max_key_fee=] - [ | --use_upnp=] - [ | --run_reflector_server=] - [ | --cache_time=] - [ | --reflect_uploads=] - [ | --share_usage_data=] - [ | --peer_search_timeout=] - [ | --sd_download_timeout=] - [ - | --auto_renew_claim_height_delta=] + [--data_rate=] + [--download_timeout=] + [--peer_port=] + [--max_key_fee=] + [--disable_max_key_fee=] + [--use_upnp=] + [--run_reflector_server=] + [--cache_time=] + [--reflect_uploads=] + [--share_usage_data=] + [--peer_search_timeout=] + [--sd_download_timeout=] + [--auto_renew_claim_height_delta=] Options: - , --download_directory= : (str) - , --data_rate= : (float), 0.0001 - , --download_timeout= : (int), 180 - , --peer_port= : (int), 3333 - , --max_key_fee= : (dict) maximum key fee for downloads, + --download_directory= : (str) + --data_rate= : (float), 0.0001 + --download_timeout= : (int), 180 + --peer_port= : (int), 3333 + --max_key_fee= : (dict) maximum key fee for downloads, in the format: { "currency": , "amount": @@ -863,16 +862,15 @@ Options: LBC BTC USD - , --disable_max_key_fee= : (bool), False - , --use_upnp= : (bool), True - , --run_reflector_server= : (bool), False - , --cache_time= : (int), 150 - , --reflect_uploads= : (bool), True - , --share_usage_data= : (bool), True - , --peer_search_timeout= : (int), 3 - , --sd_download_timeout= : (int), 3 - , - --auto_renew_claim_height_delta= : (int), 0 + --disable_max_key_fee= : (bool), False + --use_upnp= : (bool), True + --run_reflector_server= : (bool), False + --cache_time= : (int), 150 + --reflect_uploads= : (bool), True + --share_usage_data= : (bool), True + --peer_search_timeout= : (int), 3 + --sd_download_timeout= : (int), 3 + --auto_renew_claim_height_delta= : (int), 0 claims set to expire within this many blocks will be automatically renewed after startup (if set to 0, renews will not be made automatically) diff --git a/lbrynet/conf.py b/lbrynet/conf.py index 85d86d8d1..94b4eb941 100644 --- a/lbrynet/conf.py +++ b/lbrynet/conf.py @@ -8,7 +8,7 @@ import yaml import envparse from appdirs import user_data_dir, user_config_dir from lbrynet.core import utils -from lbrynet.core.Error import InvalidCurrencyError +from lbrynet.core.Error import InvalidCurrencyError, NoSuchDirectoryError from lbrynet.androidhelpers.paths import ( android_internal_storage_dir, android_app_internal_storage_dir @@ -392,9 +392,15 @@ class Config(object): if name in self._fixed_defaults: raise ValueError('{} is not an editable setting'.format(name)) - def _validate_currency(self, currency): - if currency not in self._fixed_defaults['CURRENCIES'].keys(): - raise InvalidCurrencyError(currency) + def _assert_valid_setting_value(self, name, value): + if name == "max_key_fee": + currency = str(value["currency"]).upper() + if currency not in self._fixed_defaults['CURRENCIES'].keys(): + raise InvalidCurrencyError(currency) + elif name == "download_directory": + directory = str(value) + if not os.path.exists(directory): + raise NoSuchDirectoryError(directory) def is_default(self, name): """Check if a config value is wasn't specified by the user @@ -455,11 +461,9 @@ class Config(object): data types (e.g. PERSISTED values to save to a file, CLI values from parsed command-line options, etc), you can specify that with the data_types param """ - if name == "max_key_fee": - currency = str(value["currency"]).upper() - self._validate_currency(currency) - self._assert_editable_setting(name) + self._assert_valid_setting_value(name, value) + for data_type in data_types: self._assert_valid_data_type(data_type) self._data[data_type][name] = value diff --git a/lbrynet/core/Error.py b/lbrynet/core/Error.py index 4822b7342..729ceab76 100644 --- a/lbrynet/core/Error.py +++ b/lbrynet/core/Error.py @@ -160,3 +160,8 @@ class InvalidCurrencyError(Exception): self.currency = currency Exception.__init__( self, 'Invalid currency: {} is not a supported currency.'.format(currency)) + +class NoSuchDirectoryError(Exception): + def __init__(self, directory): + self.directory = directory + Exception.__init__(self, 'No such directory {}'.format(directory)) diff --git a/lbrynet/daemon/Daemon.py b/lbrynet/daemon/Daemon.py index e65c76f9a..496743b48 100644 --- a/lbrynet/daemon/Daemon.py +++ b/lbrynet/daemon/Daemon.py @@ -453,14 +453,9 @@ class Daemon(AuthJSONRPCServer): conf.settings.update({key: decoded}, data_types=(conf.TYPE_RUNTIME, conf.TYPE_PERSISTED)) else: - try: - converted = setting_type(settings[key]) - conf.settings.update({key: converted}, - data_types=(conf.TYPE_RUNTIME, conf.TYPE_PERSISTED)) - except Exception as err: - log.warning(err.message) - log.warning("error converting setting '%s' to type %s from type %s", key, - setting_type, str(type(settings[key]))) + converted = setting_type(settings[key]) + conf.settings.update({key: converted}, + data_types=(conf.TYPE_RUNTIME, conf.TYPE_PERSISTED)) conf.settings.save_conf_file_settings() self.data_rate = conf.settings['data_rate'] @@ -1190,28 +1185,27 @@ class Daemon(AuthJSONRPCServer): Set daemon settings Usage: - settings_set [ | --download_directory=] - [ | --data_rate=] - [ | --download_timeout=] - [ | --peer_port=] - [ | --max_key_fee=] - [ | --disable_max_key_fee=] - [ | --use_upnp=] - [ | --run_reflector_server=] - [ | --cache_time=] - [ | --reflect_uploads=] - [ | --share_usage_data=] - [ | --peer_search_timeout=] - [ | --sd_download_timeout=] - [ - | --auto_renew_claim_height_delta=] + [--data_rate=] + [--download_timeout=] + [--peer_port=] + [--max_key_fee=] + [--disable_max_key_fee=] + [--use_upnp=] + [--run_reflector_server=] + [--cache_time=] + [--reflect_uploads=] + [--share_usage_data=] + [--peer_search_timeout=] + [--sd_download_timeout=] + [--auto_renew_claim_height_delta=] Options: - , --download_directory= : (str) - , --data_rate= : (float), 0.0001 - , --download_timeout= : (int), 180 - , --peer_port= : (int), 3333 - , --max_key_fee= : (dict) maximum key fee for downloads, + --download_directory= : (str) + --data_rate= : (float), 0.0001 + --download_timeout= : (int), 180 + --peer_port= : (int), 3333 + --max_key_fee= : (dict) maximum key fee for downloads, in the format: { "currency": , "amount": @@ -1221,16 +1215,15 @@ class Daemon(AuthJSONRPCServer): LBC BTC USD - , --disable_max_key_fee= : (bool), False - , --use_upnp= : (bool), True - , --run_reflector_server= : (bool), False - , --cache_time= : (int), 150 - , --reflect_uploads= : (bool), True - , --share_usage_data= : (bool), True - , --peer_search_timeout= : (int), 3 - , --sd_download_timeout= : (int), 3 - , - --auto_renew_claim_height_delta= : (int), 0 + --disable_max_key_fee= : (bool), False + --use_upnp= : (bool), True + --run_reflector_server= : (bool), False + --cache_time= : (int), 150 + --reflect_uploads= : (bool), True + --share_usage_data= : (bool), True + --peer_search_timeout= : (int), 3 + --sd_download_timeout= : (int), 3 + --auto_renew_claim_height_delta= : (int), 0 claims set to expire within this many blocks will be automatically renewed after startup (if set to 0, renews will not be made automatically)