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

Fix httpx usage so it follows redirects #808

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

claui
Copy link

@claui claui commented Dec 2, 2022

Fix httpx usage so it follows redirects

  • I read contributing guideline
  • I didn't find a similar pull request already open.
  • My PR is related to Spleeter only, not a derivative product (such as Webapplication, or GUI provided by others)

Description

Earlier today, I bumped the httpx dependency from version 0.19.0 to the latest version 0.23.1.
From Spleeter’s point of view, that version bump includes a breaking change, because since version 0.20.0, httpx no longer follows redirects by default. Instead, it treats the 3xx status family as a hard error. (See issue #686 for a real-life example.)

This PR sets the follow_redirects flag to prepare for that breaking change.

It also bumps httpx to the latest stable version in order to make sure the latest features and bugfixes are present.

Update: Fixed the description after I realized that I bumped the version of httpx.

How this patch was tested

This PR fixes the tests/test_separator.py tests, which failed reliably for me without the patch.

  • I implemented unit test whicn ran successfully using poetry run pytest tests/ (Tests already exist, see test_separator.py)
  • Code has been formatted using poetry run black spleeter
  • Imports has been formatted using `poetry run isort spleeter``

Documentation link and external references

For more details, see: https://www.python-httpx.org/compatibility/#redirects

Follow redirects when downloading from GitHub via httpx.
Fixes issue deezer#686 [1].

[1]: deezer#686

Reported-by: Antonio Petricca <antonio.petricca@gmail.com>
@claui
Copy link
Author

claui commented Mar 3, 2024

@Faylixe @romi1502 @mmoussallam @alreadytaikeune This is starting to affect others, too.

How can I help get this PR over the finish line?

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.

None yet

1 participant