From b16402e094a4f99b2f41a57d05aeaf69d5c13999 Mon Sep 17 00:00:00 2001 From: jleute Date: Thu, 17 May 2018 23:41:23 +0200 Subject: [PATCH 1/7] fixes #1109 --- CHANGELOG.md | 1 + lbrynet/conf.py | 27 +++++++++++++++++++++------ 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b94d5fd44..f149688d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ at anytime. * `use_auth_http` in a config file being overridden by the default command line argument to `lbrynet-daemon`, now the command line value will only override the config file value if it is provided * `lbrynet-cli` not automatically switching to the authenticated client if the server is detected to be using authentication. This resulted in `lbrynet-cli` failing to run when `lbrynet-daemon` was run with the `--http-auth` flag * fixed error when using `claim_show` with `txid` and `nout` arguments + * fixed error when saving server list to conf file (issue #1109) ### Deprecated * diff --git a/lbrynet/conf.py b/lbrynet/conf.py index 57dad2850..1d5d8972d 100644 --- a/lbrynet/conf.py +++ b/lbrynet/conf.py @@ -168,6 +168,8 @@ def server_port(server_and_port): def server_list(servers): return [server_port(server) for server in servers] +def server_list_reverse(servers): + return ["%s:%s" % (server, port) for server, port in servers] class Env(envparse.Env): """An Env parser that automatically namespaces the variables with LBRY""" @@ -267,7 +269,7 @@ ADJUSTABLE_SETTINGS = { 'is_generous_host': (bool, True), 'announce_head_blobs_only': (bool, True), 'concurrent_announcers': (int, DEFAULT_CONCURRENT_ANNOUNCERS), - 'known_dht_nodes': (list, DEFAULT_DHT_NODES, server_list), + 'known_dht_nodes': (list, DEFAULT_DHT_NODES, server_list, server_list_reverse), 'lbryum_wallet_dir': (str, default_lbryum_dir), 'max_connections_per_stream': (int, 5), 'seek_head_blob_first': (bool, True), @@ -285,7 +287,7 @@ ADJUSTABLE_SETTINGS = { # event the initial upload failed or was disconnected part way through, provided the auto_re_reflect_interval > 0) 'reflect_uploads': (bool, True), 'auto_re_reflect_interval': (int, 86400), # set to 0 to disable - 'reflector_servers': (list, [('reflector2.lbry.io', 5566)], server_list), + 'reflector_servers': (list, [('reflector2.lbry.io', 5566)], server_list, server_list_reverse), 'run_reflector_server': (bool, False), 'sd_download_timeout': (int, 3), 'share_usage_data': (bool, True), # whether to share usage stats and diagnostic info with LBRY @@ -295,7 +297,8 @@ ADJUSTABLE_SETTINGS = { 'use_keyring': (bool, False), 'wallet': (str, LBRYUM_WALLET), 'blockchain_name': (str, 'lbrycrd_main'), - 'lbryum_servers': (list, [('lbryum8.lbry.io', 50001), ('lbryum9.lbry.io', 50001)], server_list), + 'lbryum_servers': (list, [('lbryum8.lbry.io', 50001), ('lbryum9.lbry.io', + 50001)], server_list, server_list_reverse), 's3_headers_depth': (int, 96 * 10) # download headers from s3 when the local height is more than 10 chunks behind } @@ -497,18 +500,30 @@ class Config(object): path = conf_file else: path = self.get_conf_filename() - + # reverse the conversions done after loading the settings from the conf + # file + rev = self._convert_conf_file_lists_reverse(self._data[TYPE_PERSISTED]) ext = os.path.splitext(path)[1] encoder = settings_encoders.get(ext, False) assert encoder is not False, 'Unknown settings format %s' % ext with open(path, 'w') as settings_file: - settings_file.write(encoder(self._data[TYPE_PERSISTED])) + settings_file.write(encoder(rev)) + + @staticmethod + def _convert_conf_file_lists_reverse(converted): + rev = {} + for k in converted.iterkeys(): + if k in ADJUSTABLE_SETTINGS and len(ADJUSTABLE_SETTINGS[k]) == 4: + rev[k] = ADJUSTABLE_SETTINGS[k][3](converted[k]) + else: + rev[k] = converted[k] + return rev @staticmethod def _convert_conf_file_lists(decoded): converted = {} for k, v in decoded.iteritems(): - if k in ADJUSTABLE_SETTINGS and len(ADJUSTABLE_SETTINGS[k]) == 3: + if k in ADJUSTABLE_SETTINGS and len(ADJUSTABLE_SETTINGS[k]) >= 3: converted[k] = ADJUSTABLE_SETTINGS[k][2](v) else: converted[k] = v From 348544428fa42da2ebc00b5b7b51d24fd8534978 Mon Sep 17 00:00:00 2001 From: jleute Date: Fri, 18 May 2018 02:54:40 +0200 Subject: [PATCH 2/7] added unit test for reversal of conversion of config file entries --- lbrynet/tests/unit/test_conf.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/lbrynet/tests/unit/test_conf.py b/lbrynet/tests/unit/test_conf.py index c18ed1f83..3baa5f7ab 100644 --- a/lbrynet/tests/unit/test_conf.py +++ b/lbrynet/tests/unit/test_conf.py @@ -77,3 +77,18 @@ class SettingsTest(unittest.TestCase): self.assertEqual(str, type(conf.default_download_dir)) self.assertEqual(str, type(conf.default_data_dir)) self.assertEqual(str, type(conf.default_lbryum_dir)) + + def test_conversion_reversal(self): + # simulate decoding, conversion, conversion reversal and encoding of + # server list settings from the config file. + settings = self.get_mock_config_instance() + encoder = conf.settings_encoders['.yml'] + decoder = conf.settings_decoders['.yml'] + conf_file_entry = "lbryum_servers: ['localhost:5001', 'localhost:5002']" + decoded = decoder(conf_file_entry) + converted = settings._convert_conf_file_lists(decoded) + converted_reversed = settings._convert_conf_file_lists_reverse(converted) + encoded = encoder(converted_reversed) + self.assertEqual(conf_file_entry, encoded.strip()) + self.assertEqual(decoded, converted_reversed) + From b543c0457236ea3f8d2292e30807acc31e4dc1ad Mon Sep 17 00:00:00 2001 From: jleute Date: Sat, 19 May 2018 01:32:51 +0200 Subject: [PATCH 3/7] Updated unittest for saving of server lists in the conf file --- lbrynet/conf.py | 3 ++- lbrynet/tests/unit/test_conf.py | 29 ++++++++++++++++------------- lbrynet/tests/util.py | 4 ++++ 3 files changed, 22 insertions(+), 14 deletions(-) diff --git a/lbrynet/conf.py b/lbrynet/conf.py index 1d5d8972d..e902192fd 100644 --- a/lbrynet/conf.py +++ b/lbrynet/conf.py @@ -538,7 +538,6 @@ class Config(object): path = conf_file else: path = self.get_conf_filename() - ext = os.path.splitext(path)[1] decoder = settings_decoders.get(ext, False) assert decoder is not False, 'Unknown settings format %s' % ext @@ -645,6 +644,8 @@ def get_default_env(): for k, v in ADJUSTABLE_SETTINGS.iteritems(): if len(v) == 3: env_defaults[k] = (v[0], None, v[2]) + elif len(v) == 4: + env_defaults[k] = (v[0], None, v[2], v[3]) else: env_defaults[k] = (v[0], None) return Env(**env_defaults) diff --git a/lbrynet/tests/unit/test_conf.py b/lbrynet/tests/unit/test_conf.py index 3baa5f7ab..87d5c03fc 100644 --- a/lbrynet/tests/unit/test_conf.py +++ b/lbrynet/tests/unit/test_conf.py @@ -4,6 +4,8 @@ import json from twisted.trial import unittest from lbrynet import conf from lbrynet.core.Error import InvalidCurrencyError +from lbrynet.tests import mocks +from lbrynet.tests.util import create_conf_file class SettingsTest(unittest.TestCase): def setUp(self): @@ -78,17 +80,18 @@ class SettingsTest(unittest.TestCase): self.assertEqual(str, type(conf.default_data_dir)) self.assertEqual(str, type(conf.default_lbryum_dir)) - def test_conversion_reversal(self): - # simulate decoding, conversion, conversion reversal and encoding of - # server list settings from the config file. - settings = self.get_mock_config_instance() - encoder = conf.settings_encoders['.yml'] - decoder = conf.settings_decoders['.yml'] - conf_file_entry = "lbryum_servers: ['localhost:5001', 'localhost:5002']" - decoded = decoder(conf_file_entry) - converted = settings._convert_conf_file_lists(decoded) - converted_reversed = settings._convert_conf_file_lists_reverse(converted) - encoded = encoder(converted_reversed) - self.assertEqual(conf_file_entry, encoded.strip()) - self.assertEqual(decoded, converted_reversed) + def test_load_save_load_config_file(self): + #settings = self.get_mock_config_instance() + conf_entry = 'lbryum_servers: ["localhost:50001", "localhost:50002"]\n' + conf_temp = create_conf_file(conf_entry) + conf.conf_file = conf_temp + adjustable_settings={'data_dir': (str, conf.default_data_dir), + 'lbryum_servers': (list, [('localhost', 5001)], + conf.server_list, conf.server_list_reverse)} + env = conf.Env(**adjustable_settings) + settings = conf.Config({}, adjustable_settings, environment=env) + conf.settings = settings + settings.load_conf_file_settings() + settings.save_conf_file_settings() + settings.load_conf_file_settings() diff --git a/lbrynet/tests/util.py b/lbrynet/tests/util.py index e6ad2005c..45e4f3a15 100644 --- a/lbrynet/tests/util.py +++ b/lbrynet/tests/util.py @@ -19,6 +19,10 @@ DEFAULT_ISO_TIME = time.mktime(DEFAULT_TIMESTAMP.timetuple()) log = logging.getLogger("lbrynet.tests.util") +def create_conf_file(entry): + with tempfile.NamedTemporaryFile(delete=False, suffix='.yml') as conf: + conf.write(entry) + return conf.name def mk_db_and_blob_dir(): db_dir = tempfile.mkdtemp() From 500998b8931b092f54237301fb1481650aec2cd0 Mon Sep 17 00:00:00 2001 From: jleute Date: Sat, 19 May 2018 02:15:37 +0200 Subject: [PATCH 4/7] add assert to test_load_save_load_config_file test --- lbrynet/tests/unit/test_conf.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lbrynet/tests/unit/test_conf.py b/lbrynet/tests/unit/test_conf.py index 87d5c03fc..9159244ec 100644 --- a/lbrynet/tests/unit/test_conf.py +++ b/lbrynet/tests/unit/test_conf.py @@ -81,7 +81,6 @@ class SettingsTest(unittest.TestCase): self.assertEqual(str, type(conf.default_lbryum_dir)) def test_load_save_load_config_file(self): - #settings = self.get_mock_config_instance() conf_entry = 'lbryum_servers: ["localhost:50001", "localhost:50002"]\n' conf_temp = create_conf_file(conf_entry) conf.conf_file = conf_temp @@ -92,6 +91,8 @@ class SettingsTest(unittest.TestCase): settings = conf.Config({}, adjustable_settings, environment=env) conf.settings = settings settings.load_conf_file_settings() + first = settings.get('lbryum_servers', data_type=conf.TYPE_PERSISTED) settings.save_conf_file_settings() settings.load_conf_file_settings() - + second = settings.get('lbryum_servers', data_type=conf.TYPE_PERSISTED) + self.assertEqual(first, second) From 3add623ac9c42918693f3f48aa492285426703e4 Mon Sep 17 00:00:00 2001 From: jleute Date: Sat, 19 May 2018 02:42:45 +0200 Subject: [PATCH 5/7] clean up temporary conf file --- lbrynet/tests/unit/test_conf.py | 12 +++++++++--- lbrynet/tests/util.py | 5 ++++- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/lbrynet/tests/unit/test_conf.py b/lbrynet/tests/unit/test_conf.py index 9159244ec..aedf4f6cc 100644 --- a/lbrynet/tests/unit/test_conf.py +++ b/lbrynet/tests/unit/test_conf.py @@ -5,7 +5,7 @@ from twisted.trial import unittest from lbrynet import conf from lbrynet.core.Error import InvalidCurrencyError from lbrynet.tests import mocks -from lbrynet.tests.util import create_conf_file +from lbrynet.tests.util import create_conf_file, remove_conf_file class SettingsTest(unittest.TestCase): def setUp(self): @@ -84,7 +84,7 @@ class SettingsTest(unittest.TestCase): conf_entry = 'lbryum_servers: ["localhost:50001", "localhost:50002"]\n' conf_temp = create_conf_file(conf_entry) conf.conf_file = conf_temp - adjustable_settings={'data_dir': (str, conf.default_data_dir), + adjustable_settings = {'data_dir': (str, conf.default_data_dir), 'lbryum_servers': (list, [('localhost', 5001)], conf.server_list, conf.server_list_reverse)} env = conf.Env(**adjustable_settings) @@ -93,6 +93,12 @@ class SettingsTest(unittest.TestCase): settings.load_conf_file_settings() first = settings.get('lbryum_servers', data_type=conf.TYPE_PERSISTED) settings.save_conf_file_settings() - settings.load_conf_file_settings() + try: + settings.load_conf_file_settings() + except Exception, e: + remove_conf_file(conf_temp) + raise Exception(e) second = settings.get('lbryum_servers', data_type=conf.TYPE_PERSISTED) + remove_conf_file(conf_temp) self.assertEqual(first, second) + diff --git a/lbrynet/tests/util.py b/lbrynet/tests/util.py index 45e4f3a15..9edd70900 100644 --- a/lbrynet/tests/util.py +++ b/lbrynet/tests/util.py @@ -22,7 +22,10 @@ log = logging.getLogger("lbrynet.tests.util") def create_conf_file(entry): with tempfile.NamedTemporaryFile(delete=False, suffix='.yml') as conf: conf.write(entry) - return conf.name + return conf.name + +def remove_conf_file(filename): + os.remove(filename) def mk_db_and_blob_dir(): db_dir = tempfile.mkdtemp() From 93ee9fcff1fd7dc199011c83a0894ed4bc2c2aa2 Mon Sep 17 00:00:00 2001 From: jleute Date: Sat, 19 May 2018 09:44:15 +0200 Subject: [PATCH 6/7] remove unused import --- lbrynet/tests/unit/test_conf.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lbrynet/tests/unit/test_conf.py b/lbrynet/tests/unit/test_conf.py index aedf4f6cc..06be39600 100644 --- a/lbrynet/tests/unit/test_conf.py +++ b/lbrynet/tests/unit/test_conf.py @@ -4,7 +4,6 @@ import json from twisted.trial import unittest from lbrynet import conf from lbrynet.core.Error import InvalidCurrencyError -from lbrynet.tests import mocks from lbrynet.tests.util import create_conf_file, remove_conf_file class SettingsTest(unittest.TestCase): From 092581bf3256fa3d6130da6ee6183f8100bf7451 Mon Sep 17 00:00:00 2001 From: jleute Date: Mon, 21 May 2018 18:50:25 +0200 Subject: [PATCH 7/7] updated test to compare original and saved config file --- lbrynet/tests/unit/test_conf.py | 34 ++++++++++++++++++--------------- lbrynet/tests/util.py | 8 -------- 2 files changed, 19 insertions(+), 23 deletions(-) diff --git a/lbrynet/tests/unit/test_conf.py b/lbrynet/tests/unit/test_conf.py index 06be39600..d605bf6dc 100644 --- a/lbrynet/tests/unit/test_conf.py +++ b/lbrynet/tests/unit/test_conf.py @@ -1,10 +1,10 @@ import os import json +import tempfile from twisted.trial import unittest from lbrynet import conf from lbrynet.core.Error import InvalidCurrencyError -from lbrynet.tests.util import create_conf_file, remove_conf_file class SettingsTest(unittest.TestCase): def setUp(self): @@ -79,25 +79,29 @@ class SettingsTest(unittest.TestCase): self.assertEqual(str, type(conf.default_data_dir)) self.assertEqual(str, type(conf.default_lbryum_dir)) - def test_load_save_load_config_file(self): - conf_entry = 'lbryum_servers: ["localhost:50001", "localhost:50002"]\n' - conf_temp = create_conf_file(conf_entry) - conf.conf_file = conf_temp + def test_load_save_config_file(self): + # setup settings adjustable_settings = {'data_dir': (str, conf.default_data_dir), 'lbryum_servers': (list, [('localhost', 5001)], conf.server_list, conf.server_list_reverse)} env = conf.Env(**adjustable_settings) settings = conf.Config({}, adjustable_settings, environment=env) conf.settings = settings - settings.load_conf_file_settings() - first = settings.get('lbryum_servers', data_type=conf.TYPE_PERSISTED) - settings.save_conf_file_settings() - try: + # setup tempfile + conf_entry = "lbryum_servers: ['localhost:50001', 'localhost:50002']\n" + with tempfile.NamedTemporaryFile(suffix='.yml') as conf_file: + conf_file.write(conf_entry) + conf_file.seek(0) + conf.conf_file = conf_file.name + # load and save settings from conf file settings.load_conf_file_settings() - except Exception, e: - remove_conf_file(conf_temp) - raise Exception(e) - second = settings.get('lbryum_servers', data_type=conf.TYPE_PERSISTED) - remove_conf_file(conf_temp) - self.assertEqual(first, second) + settings.save_conf_file_settings() + # test if overwritten entry equals original entry + # use decoded versions, because format might change without + # changing the interpretation + decoder = conf.settings_decoders['.yml'] + conf_decoded = decoder(conf_entry) + conf_entry_new = conf_file.read() + conf_decoded_new = decoder(conf_entry_new) + self.assertEqual(conf_decoded, conf_decoded_new) diff --git a/lbrynet/tests/util.py b/lbrynet/tests/util.py index 9edd70900..6b9780840 100644 --- a/lbrynet/tests/util.py +++ b/lbrynet/tests/util.py @@ -19,14 +19,6 @@ DEFAULT_ISO_TIME = time.mktime(DEFAULT_TIMESTAMP.timetuple()) log = logging.getLogger("lbrynet.tests.util") -def create_conf_file(entry): - with tempfile.NamedTemporaryFile(delete=False, suffix='.yml') as conf: - conf.write(entry) - return conf.name - -def remove_conf_file(filename): - os.remove(filename) - def mk_db_and_blob_dir(): db_dir = tempfile.mkdtemp() blob_dir = tempfile.mkdtemp()