Fix issue #930. Disallow positional arguments for CLI settings_set and fix error reporting when settings_set fails.

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

docs/cli.md

@@ -832,28 +832,27 @@ Returns:
 Set daemon settings
 
 Usage:
-    settings_set [<download_directory> | --download_directory=<download_directory>]
-                 [<data_rate> | --data_rate=<data_rate>]
-                 [<download_timeout> | --download_timeout=<download_timeout>]
-                 [<peer_port> | --peer_port=<peer_port>]
-                 [<max_key_fee> | --max_key_fee=<max_key_fee>]
-                 [<disable_max_key_fee> | --disable_max_key_fee=<disable_max_key_fee>]
-                 [<use_upnp> | --use_upnp=<use_upnp>]
-                 [<run_reflector_server> | --run_reflector_server=<run_reflector_server>]
-                 [<cache_time> | --cache_time=<cache_time>]
-                 [<reflect_uploads> | --reflect_uploads=<reflect_uploads>]
-                 [<share_usage_data> | --share_usage_data=<share_usage_data>]
-                 [<peer_search_timeout> | --peer_search_timeout=<peer_search_timeout>]
-                 [<sd_download_timeout> | --sd_download_timeout=<sd_download_timeout>]
-                 [<auto_renew_claim_height_delta>
-                    | --auto_renew_claim_height_delta=<auto_renew_claim_height_delta]
+    settings_set [--download_directory=<download_directory>]
+                 [--data_rate=<data_rate>]
+                 [--download_timeout=<download_timeout>]
+                 [--peer_port=<peer_port>]
+                 [--max_key_fee=<max_key_fee>]
+                 [--disable_max_key_fee=<disable_max_key_fee>]
+                 [--use_upnp=<use_upnp>]
+                 [--run_reflector_server=<run_reflector_server>]
+                 [--cache_time=<cache_time>]
+                 [--reflect_uploads=<reflect_uploads>]
+                 [--share_usage_data=<share_usage_data>]
+                 [--peer_search_timeout=<peer_search_timeout>]
+                 [--sd_download_timeout=<sd_download_timeout>]
+                 [--auto_renew_claim_height_delta=<auto_renew_claim_height_delta>]
 
 Options:
-    <download_directory>, --download_directory=<download_directory>  : (str)
-    <data_rate>, --data_rate=<data_rate>                             : (float), 0.0001
-    <download_timeout>, --download_timeout=<download_timeout>        : (int), 180
-    <peer_port>, --peer_port=<peer_port>                             : (int), 3333
-    <max_key_fee>, --max_key_fee=<max_key_fee>   : (dict) maximum key fee for downloads,
+    --download_directory=<download_directory>  : (str)
+    --data_rate=<data_rate>                             : (float), 0.0001
+    --download_timeout=<download_timeout>        : (int), 180
+    --peer_port=<peer_port>                             : (int), 3333
+    --max_key_fee=<max_key_fee>   : (dict) maximum key fee for downloads,
                                                     in the format: {
                                                         "currency": <currency_symbol>,
                                                         "amount": <amount>
@@ -863,16 +862,15 @@ Options:
                                                         LBC
                                                         BTC
                                                         USD
-    <disable_max_key_fee>, --disable_max_key_fee=<disable_max_key_fee> : (bool), False
-    <use_upnp>, --use_upnp=<use_upnp>            : (bool), True
-    <run_reflector_server>, --run_reflector_server=<run_reflector_server>  : (bool), False
-    <cache_time>, --cache_time=<cache_time>  : (int), 150
-    <reflect_uploads>, --reflect_uploads=<reflect_uploads>  : (bool), True
-    <share_usage_data>, --share_usage_data=<share_usage_data>  : (bool), True
-    <peer_search_timeout>, --peer_search_timeout=<peer_search_timeout>  : (int), 3
-    <sd_download_timeout>, --sd_download_timeout=<sd_download_timeout>  : (int), 3
-    <auto_renew_claim_height_delta>,
-        --auto_renew_claim_height_delta=<auto_renew_claim_height_delta> : (int), 0
+    --disable_max_key_fee=<disable_max_key_fee> : (bool), False
+    --use_upnp=<use_upnp>            : (bool), True
+    --run_reflector_server=<run_reflector_server>  : (bool), False
+    --cache_time=<cache_time>  : (int), 150
+    --reflect_uploads=<reflect_uploads>  : (bool), True
+    --share_usage_data=<share_usage_data>  : (bool), True
+    --peer_search_timeout=<peer_search_timeout>  : (int), 3
+    --sd_download_timeout=<sd_download_timeout>  : (int), 3
+    --auto_renew_claim_height_delta=<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)

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

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))

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> | --download_directory=<download_directory>]
-                         [<data_rate> | --data_rate=<data_rate>]
-                         [<download_timeout> | --download_timeout=<download_timeout>]
-                         [<peer_port> | --peer_port=<peer_port>]
-                         [<max_key_fee> | --max_key_fee=<max_key_fee>]
-                         [<disable_max_key_fee> | --disable_max_key_fee=<disable_max_key_fee>]
-                         [<use_upnp> | --use_upnp=<use_upnp>]
-                         [<run_reflector_server> | --run_reflector_server=<run_reflector_server>]
-                         [<cache_time> | --cache_time=<cache_time>]
-                         [<reflect_uploads> | --reflect_uploads=<reflect_uploads>]
-                         [<share_usage_data> | --share_usage_data=<share_usage_data>]
-                         [<peer_search_timeout> | --peer_search_timeout=<peer_search_timeout>]
-                         [<sd_download_timeout> | --sd_download_timeout=<sd_download_timeout>]
-                         [<auto_renew_claim_height_delta>
-                            | --auto_renew_claim_height_delta=<auto_renew_claim_height_delta]
+            settings_set [--download_directory=<download_directory>]
+                         [--data_rate=<data_rate>]
+                         [--download_timeout=<download_timeout>]
+                         [--peer_port=<peer_port>]
+                         [--max_key_fee=<max_key_fee>]
+                         [--disable_max_key_fee=<disable_max_key_fee>]
+                         [--use_upnp=<use_upnp>]
+                         [--run_reflector_server=<run_reflector_server>]
+                         [--cache_time=<cache_time>]
+                         [--reflect_uploads=<reflect_uploads>]
+                         [--share_usage_data=<share_usage_data>]
+                         [--peer_search_timeout=<peer_search_timeout>]
+                         [--sd_download_timeout=<sd_download_timeout>]
+                         [--auto_renew_claim_height_delta=<auto_renew_claim_height_delta>]
 
         Options:
-            <download_directory>, --download_directory=<download_directory>  : (str)
-            <data_rate>, --data_rate=<data_rate>                             : (float), 0.0001
-            <download_timeout>, --download_timeout=<download_timeout>        : (int), 180
-            <peer_port>, --peer_port=<peer_port>                             : (int), 3333
-            <max_key_fee>, --max_key_fee=<max_key_fee>   : (dict) maximum key fee for downloads,
+            --download_directory=<download_directory>  : (str)
+            --data_rate=<data_rate>                             : (float), 0.0001
+            --download_timeout=<download_timeout>        : (int), 180
+            --peer_port=<peer_port>                             : (int), 3333
+            --max_key_fee=<max_key_fee>   : (dict) maximum key fee for downloads,
                                                             in the format: {
                                                                 "currency": <currency_symbol>,
                                                                 "amount": <amount>
@@ -1221,16 +1215,15 @@ class Daemon(AuthJSONRPCServer):
                                                                 LBC
                                                                 BTC
                                                                 USD
-            <disable_max_key_fee>, --disable_max_key_fee=<disable_max_key_fee> : (bool), False
-            <use_upnp>, --use_upnp=<use_upnp>            : (bool), True
-            <run_reflector_server>, --run_reflector_server=<run_reflector_server>  : (bool), False
-            <cache_time>, --cache_time=<cache_time>  : (int), 150
-            <reflect_uploads>, --reflect_uploads=<reflect_uploads>  : (bool), True
-            <share_usage_data>, --share_usage_data=<share_usage_data>  : (bool), True
-            <peer_search_timeout>, --peer_search_timeout=<peer_search_timeout>  : (int), 3
-            <sd_download_timeout>, --sd_download_timeout=<sd_download_timeout>  : (int), 3
-            <auto_renew_claim_height_delta>,
-                --auto_renew_claim_height_delta=<auto_renew_claim_height_delta> : (int), 0
+            --disable_max_key_fee=<disable_max_key_fee> : (bool), False
+            --use_upnp=<use_upnp>            : (bool), True
+            --run_reflector_server=<run_reflector_server>  : (bool), False
+            --cache_time=<cache_time>  : (int), 150
+            --reflect_uploads=<reflect_uploads>  : (bool), True
+            --share_usage_data=<share_usage_data>  : (bool), True
+            --peer_search_timeout=<peer_search_timeout>  : (int), 3
+            --sd_download_timeout=<sd_download_timeout>  : (int), 3
+            --auto_renew_claim_height_delta=<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)