Skip to content
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

Use httpx instead of aiohttp #78

Merged
merged 3 commits into from
Feb 9, 2024
Merged

Conversation

the-ress
Copy link
Contributor

@the-ress the-ress commented Jan 7, 2024

Fixes #77.

Also includes a workaround for encode/httpx#3045 and prusa3d/Prusa-Firmware-Buddy#3665. The workaround can be removed once they're resolved.

@Skaronator
Copy link
Contributor

Skaronator commented Jan 23, 2024

To get this faster merged, I think it would be helpful if you do a home-assistant PR as well

@frenck
Copy link

frenck commented Jan 23, 2024

To get this faster merged, I think it would be helpful if you do a home-assistant PR as well

That is not correct, and not wished for. See also: https://developers.home-assistant.io/docs/review-process#what-not-to-do

Do not submit new pull requests that depend on other pull requests that are still open/unmerged. This will create unneeded pull requests in the queue that are not actionable.

Thanks 👍

@Skaronator
Copy link
Contributor

Ah, sorry. Thanks for correcting frenck.

@the-ress
Copy link
Contributor Author

the-ress commented Feb 7, 2024

So what’s the process for getting this merged? Should I ping someone, or do I just wait?

pyprusalink/__init__.py Outdated Show resolved Hide resolved
@Skaronator
Copy link
Contributor

Other than that change, everything looks good and works for me on my Prusa XL with 5.1.2+13478 Firmware.

Co-authored-by: Niklas Wagner <Skaro@Skaronator.com>
@the-ress
Copy link
Contributor Author

the-ress commented Feb 8, 2024

Also for me, it works on Prusa MK4 with 5.1.2 and Prusa MK3S+ with PrusaLink 0.7.2.

Copy link
Contributor

@agners agners left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, aiohttp is generally preferred, but since digest auth is not supported (yet) httpx is the only option here.

@the-ress
Copy link
Contributor Author

the-ress commented Feb 9, 2024

Oops, I forgot to check formatting. It should be good now.

@agners agners merged commit b020946 into home-assistant-libs:main Feb 9, 2024
2 checks passed
@the-ress the-ress deleted the use_httpx branch February 9, 2024 16:03
@the-ress
Copy link
Contributor Author

the-ress commented Feb 9, 2024

Thanks, I'll try to create the core PR later today.

Edit: Assuming there'll be a new release.

@agners
Copy link
Contributor

agners commented Feb 9, 2024

Thank you for your work 🙏 🤩

Release: I'll try to make it happen 😄

@the-ress
Copy link
Contributor Author

the-ress commented Feb 18, 2024

@agners I was finally working on the core PR today, but I noticed the release to pypi failed:
https://github.com/home-assistant-libs/pyprusalink/actions/runs/7846612026
https://pypi.org/project/pyprusalink/

@agners
Copy link
Contributor

agners commented Feb 19, 2024

@the-ress ugh I see missed the version bump 🙈

I've just created a new, proper release 2.1.1, this time all green 🎉 .

@the-ress
Copy link
Contributor Author

It took me a bit longer, but here it is: home-assistant/core#114210

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Nonstandard digest auth implementation causes issues with MK3
4 participants