Handle missing settingsversion. #1747

Closed
iljah wants to merge 1 commits from patch-1 into v0.6

View File

@ -128,6 +128,8 @@ def loadConfig():
def updateConfig(): def updateConfig():
"""Save the config""" """Save the config"""
config = BMConfigParser() config = BMConfigParser()
if not config.has_option('bitmessagesettings', 'settingsversion'):
config.set('bitmessagesettings', 'settingsversion', 1)
settingsversion = config.getint('bitmessagesettings', 'settingsversion') settingsversion = config.getint('bitmessagesettings', 'settingsversion')
PeterSurda commented 2021-03-23 19:46:56 +01:00 (Migrated from github.com)
Review

Better to just add a default value into src/bmconfigparser.py

Better to just add a default value into `src/bmconfigparser.py`
iljah commented 2021-03-23 20:35:17 +01:00 (Migrated from github.com)
Review

Adding it like this at least doesn't help:

    "bitmessagesettings": {
        "settingsversion": 1,
        "maxaddrperstreamsend": 500,
Adding it like this at least doesn't help: ``` "bitmessagesettings": { "settingsversion": 1, "maxaddrperstreamsend": 500, ```
PeterSurda commented 2021-03-23 21:46:07 +01:00 (Migrated from github.com)
Review

Try using safeGetInt instead of getint in addition to adding the default value into the dict.

Try using `safeGetInt` instead of `getint` in addition to adding the default value into the dict.
iljah commented 2021-03-25 13:57:40 +01:00 (Migrated from github.com)
Review

switching to safegetint makes no difference whether settingsversion is included in BMConfigDefaults or not. No idea what the underlying cause is but my proposed fix adds only two lines...

switching to safegetint makes no difference whether settingsversion is included in BMConfigDefaults or not. No idea what the underlying cause is but my proposed fix adds only two lines...
g1itch commented 2021-03-25 16:00:07 +01:00 (Migrated from github.com)
Review
settingsversion = config.safeGetInt('bitmessagesettings', 'settingsversion', 1)

No default or the default from BMConfigDefaults is a possible bonus of python3 configparser.

Again, please read the tests: https://github.com/Bitmessage/PyBitmessage/blob/v0.6/src/tests/test_config.py#L40
Current PyBitmessage code should pass the tests.

@PeterSurda FIXME: It is also considered to be an exceptional case in current code when you open an existing config and it has no settings version. That's why getint() was retained there I think.

```python settingsversion = config.safeGetInt('bitmessagesettings', 'settingsversion', 1) ``` No default or the default from `BMConfigDefaults` is a possible bonus of python3 configparser. Again, please read the tests: https://github.com/Bitmessage/PyBitmessage/blob/v0.6/src/tests/test_config.py#L40 Current PyBitmessage code should pass the tests. @PeterSurda FIXME: It is also considered to be an exceptional case in current code when you open an existing config and it has no settings version. That's why `getint()` was retained there I think.
iljah commented 2021-03-25 16:13:29 +01:00 (Migrated from github.com)
Review

The code you give is one of the changes I tried with stated effect. To me looks like only error from tests is in Ui_MainWindow which isn't related to this as far as I can see. If you want me to make additional changes you'll have to be more specific than "rest the tests".

The code you give is one of the changes I tried with stated effect. To me looks like only error from tests is in Ui_MainWindow which isn't related to this as far as I can see. If you want me to make additional changes you'll have to be more specific than "rest the tests".
if settingsversion == 1: if settingsversion == 1:
config.set('bitmessagesettings', 'socksproxytype', 'none') config.set('bitmessagesettings', 'socksproxytype', 'none')