proxybroker2 icon indicating copy to clipboard operation
proxybroker2 copied to clipboard

NotImplementedError on Windows Python 3.8+

Open anon88391 opened this issue 1 year ago • 9 comments

Running proxybroker from CLI has been broken for me on any Python version >= 3.8

Traceback (most recent call last): File "c:\users\user\appdata\local\programs\python\python38\lib\site-packages\pycares_init_.py", line 99, in sock_state_cb sock_state_cb(socket_fd, readable, writable) File "c:\users\user\appdata\local\programs\python\python38\lib\site-packages\aiodns_init.py", line 111, in _sock_state_cb self.loop.add_reader(fd, self._handle_event, fd, READ) File "c:\users\user\appdata\local\programs\python\python38\lib\asyncio\events.py", line 501, in add_reader raise NotImplementedError

Apparently, Python 3.8 introduced a backwards-incompatible change to asyncio. https://docs.python.org/3/library/asyncio-platforms.html#asyncio-platform-support https://docs.python.org/3/library/asyncio-eventloop.html#asyncio.loop.add_reader

Some people suggested adding the following code to fix it:

import sys

if sys.platform == 'win32':
    asyncio.set_event_loop_policy(asyncio.WindowsSelectorEventLoopPolicy())
Checklist
  • [X] proxybroker/cli.py

• At the top of the file, after the import statements, add the following code:

import sys
import asyncio

if sys.platform == 'win32':
    asyncio.set_event_loop_policy(asyncio.WindowsSelectorEventLoopPolicy())

This code checks if the current platform is Windows ('win32'), and if so, it sets the event loop policy to WindowsSelectorEventLoopPolicy.

anon88391 avatar Dec 08 '22 16:12 anon88391

Didn't mean to close the issue.

Related: https://github.com/saghul/aiodns/issues/86

anon88391 avatar Dec 09 '22 11:12 anon88391

running into same problem. Any solution yet?

BukuBukuChagma avatar Dec 22 '22 11:12 BukuBukuChagma

I found a solution to this. The issue is relating to aiodns not really being maintained anymore. The last time the source code was updated was 2 years ago. I believe the current solution to this is to remove aiodns and replace it with dnspython A library which is kept more up to date.

Hopefully I will be able to make a PR fixing this issue

ziloka avatar Feb 13 '23 01:02 ziloka

Here's the PR! https://github.com/bluet/proxybroker2/pull/138.

⚡ Sweep Free Trial: I used GPT-4 to create this ticket. You have 4 GPT-4 tickets left for the month and 1 for the day. For more GPT-4 tickets, visit our payment portal. To retrigger Sweep edit the issue.


Step 1: 🔍 Code Search

I found the following snippets in your repository. I will now analyze these snippets and come up with a plan.

Some code snippets I looked at (click to expand). If some file is missing from here, you can mention the path in the ticket description.

https://github.com/bluet/proxybroker2/blob/cadfc4c589b55de481e506a14d8c7f6ab9b72cdd/proxybroker/init.py#L1-L45

https://github.com/bluet/proxybroker2/blob/cadfc4c589b55de481e506a14d8c7f6ab9b72cdd/proxybroker/data/init.py#L1-L0

https://github.com/bluet/proxybroker2/blob/cadfc4c589b55de481e506a14d8c7f6ab9b72cdd/tests/init.py#L1-L0

https://github.com/bluet/proxybroker2/blob/cadfc4c589b55de481e506a14d8c7f6ab9b72cdd/proxybroker/negotiators.py#L1-L90

https://github.com/bluet/proxybroker2/blob/cadfc4c589b55de481e506a14d8c7f6ab9b72cdd/proxybroker/api.py#L31-L147

I also found the following external resources that might be helpful:

Summaries of links found in the content:

https://docs.python.org/3/library/asyncio-eventloop.html#asyncio.loop.add_reader:

The page is the documentation for the Event Loop in Python's asyncio library. It provides information on how to work with the event loop, including methods for running and stopping the loop, scheduling callbacks, creating futures and tasks, opening network connections, creating network servers, transferring files, and more.

The page also includes examples of how to use the event loop, such as scheduling callbacks with call_soon() and call_later(), watching file descriptors for read events, setting signal handlers for SIGINT and SIGTERM, and running subprocesses.

In relation to the problem mentioned by the user, the page does not provide a direct solution. However, it does mention that asyncio ships with two different event loop implementations: SelectorEventLoop and ProactorEventLoop. By default, asyncio is configured to use SelectorEventLoop on Unix and ProactorEventLoop on Windows. The user may need to check if their code is compatible with the event loop implementation they are using.

Additionally, the page provides information on how to set the event loop policy using asyncio.set_event_loop_policy() to specify a custom event loop implementation. This may be relevant to the user's problem if they need to switch to a different event loop implementation to resolve the compatibility issue.

The code snippet suggested by some people to fix the issue involves setting the event loop policy to asyncio.WindowsSelectorEventLoopPolicy() if the platform is Windows. This may be a workaround for the compatibility issue mentioned by the user.

Overall, the page provides a comprehensive overview of the event loop in asyncio and includes examples that may be helpful in understanding and solving the problem.

https://docs.python.org/3/library/asyncio-platforms.html#asyncio-platform-support:

The page titled "Platform Support" in the Python 3.11.4 documentation discusses the platform-specific differences and limitations of the asyncio module. It mentions that the asyncio module is designed to be portable, but some platforms have subtle differences and limitations due to their underlying architecture and capabilities.

The page provides information on platform support for all platforms, Windows, and macOS. It states that on all platforms, the methods loop.add_reader() and loop.add_writer() cannot be used to monitor file I/O.

For Windows, it mentions that the ProactorEventLoop is now the default event loop starting from version 3.8. It also lists the methods and features that are not supported on Windows, such as loop.create_unix_connection(), loop.create_unix_server(), loop.add_signal_handler(), and loop.remove_signal_handler(). It further explains the limitations of the SelectorEventLoop and ProactorEventLoop on Windows.

Regarding macOS, it states that modern macOS versions are fully supported. However, on macOS versions 10.6, 10.7, and 10.8, the default event loop uses selectors.KqueueSelector, which does not support character devices on these versions. It provides an example of manually configuring the SelectorEventLoop to use SelectSelector or PollSelector to support character devices on these older versions of macOS.

In summary, the page provides information on the platform-specific differences and limitations of the asyncio module, including details about support on all platforms, Windows, and macOS. It does not specifically address the issue mentioned by the user, but it may provide some insights into the problem. Additionally, the page does not include the suggested code snippet to fix the issue.


Step 2: 🧐 Snippet Analysis

From looking through the relevant snippets, I decided to make the following modifications:

File Path Proposed Changes
proxybroker/cli.py Modify proxybroker/cli.py with contents:
• At the top of the file, after the import statements, add the following code:
```python
import sys
import asyncio

if sys.platform == 'win32':
asyncio.set_event_loop_policy(asyncio.WindowsSelectorEventLoopPolicy())
```
This code checks if the current platform is Windows ('win32'), and if so, it sets the event loop policy to WindowsSelectorEventLoopPolicy.

Step 3: 📝 Planning

I have created a plan for writing the pull request. I am now working my plan and coding the required changes to address this issue. Here is the planned pull request:

Fix NotImplementedError on Windows Python 3.8+ sweep/fix-notimplementederror-windows-python38

Description

This PR fixes the NotImplementedError that occurs when running ProxyBroker from the command line interface (CLI) on Windows with Python 3.8 or higher. The issue is caused by a change in the default event loop policy in Python 3.8 on Windows, which does not support the add_reader method used by ProxyBroker. The fix sets the event loop policy to WindowsSelectorEventLoopPolicy on Windows to resolve the compatibility issue.

Summary of Changes

  • Added code to set the event loop policy to WindowsSelectorEventLoopPolicy in the proxybroker/cli.py file.
  • The code is placed at the top of the file, after the import statements.
  • The fix is only applied on Windows, as the issue does not occur on other platforms.
  • The fix ensures that the add_reader method is supported by the event loop policy, resolving the NotImplementedError issue.

Step 4: ⌨️ Coding

File Instructions Progress
proxybroker/cli.py Modify proxybroker/cli.py with contents:
• At the top of the file, after the import statements, add the following code:
python<br/> import sys<br/> import asyncio<br/><br/> if sys.platform == 'win32':<br/> asyncio.set_event_loop_policy(asyncio.WindowsSelectorEventLoopPolicy())<br/>
This code checks if the current platform is Windows ('win32'), and if so, it sets the event loop policy to WindowsSelectorEventLoopPolicy.
✅ Commit 73eb60a

Step 5: 🔁 Code Review

Here are my self-reviews of my changes at sweep/fix-notimplementederror-windows-python38.

Here is the 1st review

Thanks for your contribution. There's just a small change needed:

  • In proxybroker/cli.py, you've imported sys twice, once at the beginning of the file and again in the added lines (7-12). You only need to import a module once, so you can remove the second import.

Here's how you can update it:

import sys
from contextlib import contextmanager
import asyncio

if sys.platform == 'win32':
    asyncio.set_event_loop_policy(asyncio.WindowsSelectorEventLoopPolicy())

from . import __version__ as version
from .api import Broker
from .utils import update_geoip_db

Keep up the good work!

I finished incorporating these changes.


To recreate the pull request edit the issue title or description. Join Our Discord

sweep-ai[bot] avatar Aug 17 '23 23:08 sweep-ai[bot]

@bluet The issue doesn't seem fixed with #138 at all. The exception isn't raised anymore, but the whole program seems to be doing absolutely nothing after this change - it just hangs doing literally nothing. Did you seriously ask GPT4 to fix it and then pushed its code without verifying if it works?

anon88391 avatar Sep 08 '23 12:09 anon88391

@anon88391 the fix was based on your suggestion. I don't have Windows machines for decades, and as you do and suggestions that, I applied it.

bluet avatar Sep 09 '23 03:09 bluet

Seems like aiodns' back in the game https://github.com/saghul/aiodns/tags

bluet avatar Dec 06 '23 15:12 bluet

Seems like aiodns' back in the game https://github.com/saghul/aiodns/tags

yes, I updated the pull request just to update aiodns and aiohttp dependencies.

just need to fix a unit test that broke

ziloka avatar Dec 06 '23 18:12 ziloka

@ziloka I just fixed the error in the unit test and dumped dependencies. Not sure about windows support.

bluet avatar Dec 07 '23 11:12 bluet