-
-
Notifications
You must be signed in to change notification settings - Fork 30.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Swap octoprint to use an external library #46611
Swap octoprint to use an external library #46611
Conversation
08f362e
to
b0d322f
Compare
b0d322f
to
ec786bf
Compare
4f0c257
to
2e8bc35
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
The code seems to be failing on import homekit config. Any pointers in hunting down what broke? |
Most likely a flaky test unrelated to this PR. I restarted CI. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of small comments. All-in-all, good job!
First attempt with an invalid host resulted in:
A second attempt at my Octoprint dev instance (which has no permanent printer connected):
Third attempt with a printer actively connected worked. |
Both situations should be fixed with more reasonable exception messages into the log file. Updated the data update to only catch the printer offline exception and let anything from the getjob get raised. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Proposed change
To prepare to switching octoprint over to config_flow, move the API code out into a library.
I used a client that has wrapper classes around the DTOs, which required a few other changes. If the HA teams prefers the client return json instead, I can update to do that.
Type of change
Example entry for
configuration.yaml
:# Example configuration.yaml
Additional information
Checklist
black --fast homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
..coveragerc
.The integration reached or maintains the following Integration Quality Scale:
To help with the load of incoming pull requests: