Skip to content

Conversation

@gautaz
Copy link
Contributor

@gautaz gautaz commented Apr 7, 2025

Hello,

Currently yewtube does not support httpx>=0.28.0, see:
#394572

These changes ensure that http=0.27.2 is used in order to work around this issue.

I am new to Python packages overriding, so if I am doing it wrong, please advise.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Apr 7, 2025
@nix-owners nix-owners bot requested a review from fgaz April 7, 2025 12:02
@drupol
Copy link
Contributor

drupol commented Apr 7, 2025

Hello,

Thanks for your PR, but I can't help but think that this kind of issue should be fixed upstream, not downstream. Perhaps I'm wrong here, but it feels weird to do that to fix something that should be fixed upstream.

@gautaz
Copy link
Contributor Author

gautaz commented Apr 7, 2025

Hello,

Thanks for your PR, but I can't help but think that this kind of issue should be fixed upstream, not downstream. Perhaps I'm wrong here, but it feels weird to do that to fix something that should be fixed upstream.

I agree, currently the issue was closed on yewtube side, I was planning to ask for reopening it.

But in the meantime, as there is a workaround, don't you think that it would be beneficial for all other nixpkgs users to at least have the opportunity to use yewtube?

This is currently necessary to work around the following yewtube issue:
mps-youtube/yewtube#1303
@FliegendeWurst
Copy link
Member

FliegendeWurst commented Apr 7, 2025

Would be better to fix the root cause here: https://github.com/alexmercerind/youtube-search-python/blob/main/youtubesearchpython/core/requests.py#L25

Using the patch below. Code adapted using information from encode/httpx#2879

diff --git a/youtubesearchpython/core/requests.py b/youtubesearchpython/core/requests.py
index ea3ed7f..3eaa2a3 100644
--- a/youtubesearchpython/core/requests.py
+++ b/youtubesearchpython/core/requests.py
@@ -11,10 +11,10 @@ class RequestCore:
         self.proxy = {}
         http_proxy = os.environ.get("HTTP_PROXY")
         if http_proxy:
-            self.proxy["http://"] = http_proxy
+            self.proxy["http://"] = httpx.HTTPTransport(proxy=http_proxy)
         https_proxy = os.environ.get("HTTPS_PROXY")
         if https_proxy:
-            self.proxy["https://"] = https_proxy
+            self.proxy["https://"] = httpx.HTTPTransport(proxy=https_proxy)

     def syncPostRequest(self) -> httpx.Response:
         return httpx.post(
@@ -22,18 +22,18 @@ class RequestCore:
             headers={"User-Agent": userAgent},
             json=self.data,
             timeout=self.timeout,
-            proxies=self.proxy
+            mounts=self.proxy
         )

     async def asyncPostRequest(self) -> httpx.Response:
-        async with httpx.AsyncClient(proxies=self.proxy) as client:
+        async with httpx.AsyncClient(mounts=self.proxy) as client:
             r = await client.post(self.url, headers={"User-Agent": userAgent}, json=self.data, timeout=self.timeout)
             return r

     def syncGetRequest(self) -> httpx.Response:
-        return httpx.get(self.url, headers={"User-Agent": userAgent}, timeout=self.timeout, cookies={'CONSENT': 'YES+1'}, proxies=self.proxy)
+        return httpx.get(self.url, headers={"User-Agent": userAgent}, timeout=self.timeout, cookies={'CONSENT': 'YES+1'}, mounts=self.proxy)

     async def asyncGetRequest(self) -> httpx.Response:
-        async with httpx.AsyncClient(proxies=self.proxy) as client:
+        async with httpx.AsyncClient(mounts=self.proxy) as client:
             r = await client.get(self.url, headers={"User-Agent": userAgent}, timeout=self.timeout, cookies={'CONSENT': 'YES+1'})
             return r

Save as fix-httpx-proxies.patch and add to youtube-search-python's patches

@FliegendeWurst
Copy link
Member

Note: the patch above cannot be upstreamed, since the corresponding repository is archived.

@drupol
Copy link
Contributor

drupol commented Apr 7, 2025

I definitely prefer the solution proposed by @FliegendeWurst ! (CC @amozeo)

@gautaz
Copy link
Contributor Author

gautaz commented Apr 7, 2025

Note: the patch above cannot be upstreamed, since the corresponding repository is archived.

Ok, that explains why the current fix proposed on yewtube side is downgrading httpx.
In order to push the patch upstream, it would probably mean for the yewtube maintainers to take over the project youtube-search-python, I'll see how this goes on their side.

In the meantime (and it will probably take long on yewtube side), what is the best course of actions on nixpkgs side?
@FliegendeWurst Are you actually meaning to apply this patch on nixpkgs side in the youtube-search-python package?

@FliegendeWurst
Copy link
Member

Are you actually meaning to apply this patch on nixpkgs side in the youtube-search-python package?

Yes, this will benefit other users of the package too. Save the patch in pkgs/development/python-modules/youtube-search-python/fix-httpx-proxies.patch, and add this to youtube-search-python's default.nix

patches = [
  # Update usage of deprecated and removed proxies argument.
  # https://github.com/encode/httpx/pull/2879
  # https://github.com/NixOS/nixpkgs/pull/396811
  ./fix-httpx-proxies.patch
];

@gautaz
Copy link
Contributor Author

gautaz commented Apr 7, 2025

Yes, this will benefit other users of the package too. Save the patch in pkgs/development/python-modules/youtube-search-python/fix-httpx-proxies.patch, and add this to youtube-search-python's default.nix

Ok, I'll create a new PR for this and will close this one when the new one is ready.
I'll keep you posted.

@drupol
Copy link
Contributor

drupol commented Apr 7, 2025

You can do everything within this PR.

@amozeo
Copy link
Contributor

amozeo commented Apr 7, 2025

I don't use yewtube package myself. I like the proposed solution of patching youtube-search-python.

@gautaz
Copy link
Contributor Author

gautaz commented Apr 7, 2025

You can do everything within this PR.

@drupol I just saw your message, I have already started a new branch, my apologies.

Seems like the patch for youtube-search-python needs additional changes, I got this while testing with yewtube:

post() got an unexpected keyword argument 'mounts'

I am looking into it.

@gautaz
Copy link
Contributor Author

gautaz commented Apr 7, 2025

It seems the right patch would be:

--- a/youtubesearchpython/core/requests.py
+++ b/youtubesearchpython/core/requests.py
@@ -11,10 +11,10 @@ class RequestCore:
         self.proxy = {}
         http_proxy = os.environ.get("HTTP_PROXY")
         if http_proxy:
-            self.proxy["http://"] = http_proxy
+            self.proxy["http://"] = httpx.HTTPTransport(proxy=http_proxy)
         https_proxy = os.environ.get("HTTPS_PROXY")
         if https_proxy:
-            self.proxy["https://"] = https_proxy
+            self.proxy["https://"] = httpx.HTTPTransport(proxy=https_proxy)

     def syncPostRequest(self) -> httpx.Response:
         return httpx.post(
@@ -22,18 +22,17 @@ class RequestCore:
             headers={"User-Agent": userAgent},
             json=self.data,
             timeout=self.timeout,
-            proxies=self.proxy
         )

     async def asyncPostRequest(self) -> httpx.Response:
-        async with httpx.AsyncClient(proxies=self.proxy) as client:
+        async with httpx.AsyncClient(mounts=self.proxy) as client:
             r = await client.post(self.url, headers={"User-Agent": userAgent}, json=self.data, timeout=self.timeout)
             return r

     def syncGetRequest(self) -> httpx.Response:
-        return httpx.get(self.url, headers={"User-Agent": userAgent}, timeout=self.timeout, cookies={'CONSENT': 'YES+1'}, proxies=self.proxy)
+        return httpx.get(self.url, headers={"User-Agent": userAgent}, timeout=self.timeout, cookies={'CONSENT': 'YES+1'}, mounts=self.proxy)

     async def asyncGetRequest(self) -> httpx.Response:
-        async with httpx.AsyncClient(proxies=self.proxy) as client:
+        async with httpx.AsyncClient(mounts=self.proxy) as client:
             r = await client.get(self.url, headers={"User-Agent": userAgent}, timeout=self.timeout, cookies={'CONSENT': 'YES+1'})
             return r

yewtube seems to behave well with this one.
I'll push the PR as soon as it's ready.

@FliegendeWurst
Copy link
Member

I think proxy=self.proxy.get("https://") would be more correct for httpx.post and httpx.get calls

@FliegendeWurst
Copy link
Member

Though it may be safer to just remove the proxies argument, yes. Certainly easier to test.

@gautaz
Copy link
Contributor Author

gautaz commented Apr 7, 2025

And I took the time to read the httpx docs, so the right patch is finally:

--- a/youtubesearchpython/core/requests.py
+++ b/youtubesearchpython/core/requests.py
@@ -11,29 +11,28 @@ class RequestCore:
         self.proxy = {}
         http_proxy = os.environ.get("HTTP_PROXY")
         if http_proxy:
-            self.proxy["http://"] = http_proxy
+            self.proxy["http://"] = httpx.HTTPTransport(proxy=http_proxy)
         https_proxy = os.environ.get("HTTPS_PROXY")
         if https_proxy:
-            self.proxy["https://"] = https_proxy
+            self.proxy["https://"] = httpx.HTTPTransport(proxy=https_proxy)

     def syncPostRequest(self) -> httpx.Response:
-        return httpx.post(
+        return httpx.Client(mounts=self.proxy).post(
             self.url,
             headers={"User-Agent": userAgent},
             json=self.data,
-            timeout=self.timeout,
-            proxies=self.proxy
+            timeout=self.timeout
         )

     async def asyncPostRequest(self) -> httpx.Response:
-        async with httpx.AsyncClient(proxies=self.proxy) as client:
+        async with httpx.AsyncClient(mounts=self.proxy) as client:
             r = await client.post(self.url, headers={"User-Agent": userAgent}, json=self.data, timeout=self.timeout)
             return r

     def syncGetRequest(self) -> httpx.Response:
-        return httpx.get(self.url, headers={"User-Agent": userAgent}, timeout=self.timeout, cookies={'CONSENT': 'YES+1'}, proxies=self.proxy)
+        return httpx.Client(mounts=self.proxy).get(self.url, headers={"User-Agent": userAgent}, timeout=self.timeout, cookies={'CONSENT': 'YES+1'})

     async def asyncGetRequest(self) -> httpx.Response:
-        async with httpx.AsyncClient(proxies=self.proxy) as client:
+        async with httpx.AsyncClient(mounts=self.proxy) as client:
             r = await client.get(self.url, headers={"User-Agent": userAgent}, timeout=self.timeout, cookies={'CONSENT': 'YES+1'})
             return r

I should have started with this in the first place 🤣.

@gautaz
Copy link
Contributor Author

gautaz commented Apr 7, 2025

The new pull request is available, see #396845.
I am closing this one.

@gautaz gautaz closed this Apr 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants