Skip to content
This repository was archived by the owner on Jan 27, 2023. It is now read-only.

resolve KeyError#33

Closed
LukeAllen wants to merge 1 commit into
seveas:masterfrom
LukeAllen:master
Closed

resolve KeyError#33
LukeAllen wants to merge 1 commit into
seveas:masterfrom
LukeAllen:master

Conversation

@LukeAllen

Copy link
Copy Markdown

When I called 'import NetworkManager', I observed a KeyError on these lines, caused by an arg that had a 'type' attribute but not a 'name' attribute.

I don't fully understand this section of code, but I tried debugging a bit. When the KeyError occurred, I printed the other attributes on that line:

Other values when arg.attrib['name'] raised a KeyError:

arg.attrib['type'] = 'o'
element.attrib['name'] = 'org.freedesktop.NetworkManager'
item.attrib['name'] = 'DeviceRemoved'

arg.attrib['type'] = 'o'
element.attrib['name'] = 'org.freedesktop.NetworkManager'
item.attrib['name'] = 'DeviceAdded'

arg.attrib['type'] = 'a{sv}'
element.attrib['name'] = 'org.freedesktop.NetworkManager'
item.attrib['name'] = 'PropertiesChanged'

arg.attrib['type'] = 'u'
element.attrib['name'] = 'org.freedesktop.NetworkManager'
item.attrib['name'] = 'StateChanged'

arg.attrib['type'] = 'o'
element.attrib['name'] = 'org.freedesktop.NetworkManager.Settings'
item.attrib['name'] = 'ConnectionRemoved'

arg.attrib['type'] = 'o'
element.attrib['name'] = 'org.freedesktop.NetworkManager.Settings'
item.attrib['name'] = 'NewConnection'

arg.attrib['type'] = 'a{sv}'
element.attrib['name'] = 'org.freedesktop.NetworkManager.Settings'
item.attrib['name'] = 'PropertiesChanged'

The problems occurred on Raspbian when wlan0 was not connected to an access point, and wlan1 was connected. The edits in this PR fixed it, and NetworkManager was still able to control connections, at least to the extent I could test.

(Also, please let me know if there's a better way to handle this problem than setting name to 'unknown'. I'm new to dbus and NetworkManager.)

@seveas

seveas commented Mar 14, 2017

Copy link
Copy Markdown
Owner

That solution would be quite wrong. This is probably a duplicate of #32, which isn't really solvable. Which NetworkManager version are you using?

@LukeAllen

Copy link
Copy Markdown
Author

Hey, thanks for getting back to me. Yeah it's definitely the same issue. nmcli -v says the version = 0.9.10.0, but the newer versions of Network Manager aren't available from the Debian package repo unfortunately. So I'd like my program to support the current default package. What do you think is the best way to handle the missing key? Just skip that entry maybe, or skip it and print a warning?

Those entries seem pretty non-critical. The approach of "do something decently reasonable and print a warning" is how a lot of Python packages (e.g. numpy) warn you that you're using deprecated behavior. Should I change my PR to do that?

@seveas

seveas commented Mar 15, 2017

Copy link
Copy Markdown
Owner

Those entries are entirely critical, without them you can't handle signals...

@LukeAllen

Copy link
Copy Markdown
Author

The current behavior of crashing the Python interpreter also removes my ability to handle signals.

The other option I can think of is to throw a catchable exception at the end of the import, to warn of the missing functionality. That's just a thought though. What do you think is the correct workaround on distros with older network manager versions?

@seveas

seveas commented Mar 18, 2017

Copy link
Copy Markdown
Owner

Try 522658b

@LukeAllen

Copy link
Copy Markdown
Author

Thanks for working on it. I just tested. That fixed one KeyError, but there's also a KeyError: 'name' from line 237:

SignalDispatcher.args[(element.attrib['name'], item.attrib['name'])] = [(arg.attrib['name'], arg.attrib['type']) for arg in item]

I think a similar code change to line 237 should fix the problem.

@seveas

seveas commented Mar 20, 2017 via email

Copy link
Copy Markdown
Owner

@LukeAllen

Copy link
Copy Markdown
Author

Thanks, I just tested and that fixed it. I'm happy closing this PR when you are.

@seveas seveas closed this Apr 4, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants