Skip to content

Handle missing settingsversion. #1747

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
iljah wants to merge 1 commit into from
Closed

Handle missing settingsversion. #1747

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/helper_startup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,8 @@ def loadConfig():
def updateConfig():
"""Save the config"""
config = BMConfigParser()
if not config.has_option('bitmessagesettings', 'settingsversion'):
config.set('bitmessagesettings', 'settingsversion', 1)
settingsversion = config.getint('bitmessagesettings', 'settingsversion')
Copy link
Member

@PeterSurda PeterSurda Mar 23, 2021

Choose a reason for hiding this comment

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

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

Copy link
Author

@iljah iljah Mar 23, 2021

Choose a reason for hiding this comment

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

Adding it like this at least doesn't help:

    "bitmessagesettings": {
        "settingsversion": 1,
        "maxaddrperstreamsend": 500,

Copy link
Member

@PeterSurda PeterSurda Mar 23, 2021

Choose a reason for hiding this comment

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

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

Copy link
Author

@iljah iljah Mar 25, 2021

Choose a reason for hiding this comment

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

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

Copy link
Collaborator

@g1itch g1itch Mar 25, 2021

Choose a reason for hiding this comment

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

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.

Copy link
Author

@iljah iljah Mar 25, 2021

Choose a reason for hiding this comment

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

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:
config.set('bitmessagesettings', 'socksproxytype', 'none')
Expand Down