Octoprint 2.0.0 fixes#267
Open
jacopotediosi wants to merge 14 commits into
Open
Conversation
Since OctoPrint 1.10.0 the `destinations` argument has been renamed to `locations`. To be compatible with all versions, the best way is to avoid using argument by keyword at all.
Accessing `self._printer._comm` has never been supported. The correct way is to use only the documented hooks, in this case `octoprint.comm.protocol.gcode.sending`.
`n` was returned before its initialization
`qdata` doesn't exist, the parameter is called `a`.
`imp` doesn't exist anymore in recent Python versions. `importlib` is the right replacement and works on all Python versions.
OctoPrint ships PNotify v2, which by default doesn't escape HTML from `text` field, exposing notifications to potential XSS attacks.
This was done via the "octoprint dev plugin:migrate-to-pyproject" command, with minor manual adjustments to the generated files. Ref: https://docs.octoprint.org/en/dev/plugins/migration_pyproject_toml.html
So they can then be run with the single command `python -m pytest`. Currently, all tests pass.
Author
|
@smartin015 When you have time, could you please take a look at this one? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hi, I'm an active contributor to OctoPrint core. I helped ship OctoPrint 2.0.0rc1 and I'm now helping plugins stay compatible with the new and upcoming OctoPrint releases, which is why you're receiving this PR.
What this PR does
The main goal of this PR is to fix multiple compatibility issues and usages of long-deprecated features that would cause the plugin to malfunction - or fail to install altogether - on OctoPrint 2.0.0rc1.
In particular:
destinationskwarg by name toFileManager.list_files(), since it has been renamed in OctoPrint 2.0.0. It has always been intended to use as a positional argument.octoprint.comm.protocol.gcode.sendinghook (ref) instead of monkey-patching OctoPrint internals. This plugin codebase was accessingself._printer._comm, a private attribute that was never meant to be used by plugins (ref).is_blueprint_csrf_protected(ref).TemplatePluginautoescape (ref).text_escape: trueis passed among the initialization args.pyproject(ref). This was done via theoctoprint dev plugin:migrate-to-pyprojectcommand, with minor manual adjustments to the generated files.While working on this PR I also had to deal with some pre-existing issues in the codebase, so the following minor changes are included as well:
_cleanup_filesharewhere a variable was returned before being initialized.Review suggestions
Each change is isolated in its own commit, so I suggest reviewing this PR commit-by-commit rather than as a single diff. If merged, I'd also suggest keeping the commits separate so individual changes can be reverted if any of them turn out to be problematic.
Testing
I tested the installation on both OctoPrint 2.0.0rc1 and the latest stable release 1.11.7, and both succeed. The plugin UI looks fine. All unit tests pass without errors. Please note that I haven't actually tested the plugin against a real print, since I'm not a user of this plugin and don't have a setup ready - so real-world testing is up to you. That said, given the relative simplicity of the changes I made, I'm fairly confident everything still works as expected.