Fix: capture TypeError caused by dynamic reloading on settings save and load

Usage of super(MyClass, self) is henceforth discouraged due to dynamic reloading within OctoPrint's plugin subsystem. Refer to https://thingspython.wordpress.com/2010/09/27/another-super-wrinkle-raising-typeerror/ for details on this problem.

Action by plugin authors is needed to remove the super(...) call and substitute it with octoprint.plugin.SettingsPlugin.on_settings_save(self, data) and octoprint.plugin.SettingsPlugin.on_settings_load(self). Without these modifications, calls to plugin methods utilizing super(...) will fail after a call to the plugin managers reload_plugins method.

Closes #942
This commit is contained in:
Gina Häußge 2015-06-19 13:17:47 +02:00
parent 0846a0fed0
commit 633d1ae594
2 changed files with 26 additions and 7 deletions

View file

@ -718,7 +718,7 @@ class SettingsPlugin(OctoPrintPlugin):
def on_settings_save(self, data):
old_flag = self._settings.get_boolean(["sub", "some_flag"])
super(MySettingsPlugin, self).on_settings_save(data)
octoprint.plugin.SettingsPlugin.on_settings_save(self, data)
new_flag = self._settings.get_boolean(["sub", "some_flag"])
if old_flag != new_flag:

View file

@ -26,6 +26,8 @@ import octoprint.util
@api.route("/settings", methods=["GET"])
def getSettings():
logger = logging.getLogger(__name__)
s = settings()
connectionOptions = get_connection_options()
@ -118,7 +120,7 @@ def getSettings():
for name in gcode_scripts:
data["scripts"]["gcode"][name] = s.loadScript("gcode", name, source=True)
def process_plugin_result(name, plugin, result):
def process_plugin_result(name, result):
if result:
if not "plugins" in data:
data["plugins"] = dict()
@ -126,9 +128,17 @@ def getSettings():
del result["__enabled"]
data["plugins"][name] = result
octoprint.plugin.call_plugin(octoprint.plugin.SettingsPlugin,
"on_settings_load",
callback=process_plugin_result)
for plugin in octoprint.plugin.plugin_manager().get_implementations(octoprint.plugin.SettingsPlugin):
try:
result = plugin.on_settings_load()
process_plugin_result(plugin._identifier, result)
except TypeError:
logger.warn("Could not load settings for plugin {name} ({version}) since it called super(...)".format(name=plugin._plugin_name, version=plugin._plugin_version))
logger.warn("in a way which has issues due to OctoPrint's dynamic reloading after plugin operations.")
logger.warn("Please contact the plugin's author and ask to update the plugin to use a direct call like")
logger.warn("octoprint.plugin.SettingsPlugin.on_settings_load(self) instead.")
except:
logger.exception("Could not load settings for plugin {name} ({version})".format(version=plugin._plugin_version, name=plugin._plugin_name))
return jsonify(data)
@ -137,6 +147,8 @@ def getSettings():
@restricted_access
@admin_permission.require(403)
def setSettings():
logger = logging.getLogger(__name__)
if not "application/json" in request.headers["Content-Type"]:
return make_response("Expected content-type JSON", 400)
@ -235,8 +247,15 @@ def setSettings():
for plugin in octoprint.plugin.plugin_manager().get_implementations(octoprint.plugin.SettingsPlugin):
plugin_id = plugin._identifier
if plugin_id in data["plugins"]:
plugin.on_settings_save(data["plugins"][plugin_id])
try:
plugin.on_settings_save(data["plugins"][plugin_id])
except TypeError:
logger.warn("Could not save settings for plugin {name} ({version}) since it called super(...)".format(name=plugin._plugin_name, version=plugin._plugin_version))
logger.warn("in a way which has issues due to OctoPrint's dynamic reloading after plugin operations.")
logger.warn("Please contact the plugin's author and ask to update the plugin to use a direct call like")
logger.warn("octoprint.plugin.SettingsPlugin.on_settings_save(self, data) instead.")
except:
logger.exception("Could not save settings for plugin {name} ({version})".format(version=plugin._plugin_version, name=plugin._plugin_name))
if s.save():
eventManager().fire(Events.SETTINGS_UPDATED)