seveas / python-networkmanager

Easy communication with NetworkManager
http://packages.python.org/python-networkmanager/
Other
166 stars 90 forks source link

Allow configuration of `follow_name_owner_changes` #90

Closed maggie44 closed 1 year ago

maggie44 commented 3 years ago

The follow_name_owner_changes boolean default was changed to True to resolve a permissions issue when NetworkManager restarts https://github.com/seveas/python-networkmanager/issues/66. This subsequently introduced some new issues:

It would seem that the follow_name_owner_changes default would be better set to False to resolve the issues above, but also as it is the DBus default (https://dbus.freedesktop.org/doc/dbus-python/dbus.bus.html) and because it has been tried and tested as False for users for many versions (if it isn't broke, don't fix it). The reversion to True has been a breaking change for users unnecessarily.

As outlined in https://github.com/seveas/python-networkmanager/issues/66 there are some use cases where someone may need the follow_name_owner_changes to be True (although outnumbered considerably by those current experiencing issues). Therefore, this commit changes the follow_name_owner_changes back to False, but provides the ability for a user to change the value of the Boolean.

After this commit:

Use with follow_name_owner_changes = False (as before the breaking change):

import NetworkManager

Use with follow_name_owner_changes = True:

from dbus.mainloop.glib import DBusGMainLoop
DBusGMainLoop(set_as_default=True)

import NetworkManager
NetworkManager.Config(follow_name_owner_changes=True)

This approach is inspired by https://github.com/seveas/python-networkmanager/pull/85, which while resolving the issue forces the setting of follow_name_owner_changes under certain circumstances. Introducing the ability to set custom variables may prove more logical to users, provides room for expansion of other variables in the future, and resolves the breaking changes.

The 24 hour drop of connection issue https://github.com/seveas/python-networkmanager/issues/87 may still need to be addressed for when the config value is changed to True, however this would be better treated as a separate issue to resolve under those certain circumstances, rather than imposing the less tested change of the Boolean on all users. Hopefully it will also mean we can merge this initial fix more quickly to restore the use of Python NetworkManager without having to wait for a resolution of the more complex https://github.com/seveas/python-networkmanager/issues/87.

Further testing appreciated and changes/suggestions welcome.

seveas commented 1 year ago

Closing all PR's and issues prior to archiving this repository.