Skip to content

Commit

Permalink
Fix: capture TypeError caused by dynamic reloading on settings save a…
Browse files Browse the repository at this point in the history
…nd 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
  • Loading branch information
foosel committed Jun 19, 2015
1 parent 0846a0f commit 633d1ae
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 7 deletions.
2 changes: 1 addition & 1 deletion src/octoprint/plugin/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -718,7 +718,7 @@ def get_settings_defaults(self):
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:
Expand Down
31 changes: 25 additions & 6 deletions src/octoprint/server/api/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@

@api.route("/settings", methods=["GET"])
def getSettings():
logger = logging.getLogger(__name__)

s = settings()

connectionOptions = get_connection_options()
Expand Down Expand Up @@ -118,17 +120,25 @@ 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()
if "__enabled" in result:
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)

Expand All @@ -137,6 +147,8 @@ def process_plugin_result(name, plugin, result):
@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)

Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 633d1ae

Please sign in to comment.