-
Notifications
You must be signed in to change notification settings - Fork 165
add winloop for windows if use_uvloop is used on windows #960
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
base: master
Are you sure you want to change the base?
Conversation
I specifically asked on Gitter to use |
I also mentioned augmenting the |
@agronholm I'm fixing the issue. one moment I added winloop to pyproject.toml I just forgot to register it. As for the flag I'm reverting that idea. |
@agronholm I'll make sure to grab what I previously had for documentation about winloop real quick. |
Do you intend to finish this PR? |
Yeah I was waiting on you to review or see if changes were required seems 3.9 & 3.13 are failing for some unknown reason. |
There's no point in asking for review if the tests don't pass. We already discussed the necessary changes on Gitter / Matrix. The DNS resolution needs the Windows special casing turned off when run with winloop, and as for the |
I will approve this once you change the test skipping to use the same method as with uvloop. It's okay to add a utility function in the test suite to check for winloop/uvloop as you would now need to duplicate the same nontrivial logic. |
Oh, and do add that changelog entry which you already had in your previous PR. |
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
is there anything left or is this mergeable? |
Why is there an added winloop skip in |
I need the above question resolved before I merge. |
@agronholm there seems to be a bug that doesn't want it to pass and seems to be a problem with the library itself maybe. I'll see about making a duplicate of the test but basically the one that is failing is tests/test_subprocesses.py::test_run_process[asyncio+uvloop-shell] for some strange reason. I'll see about patching it in a later update. |
I'd really like to see this fixed in winloop before this PR is merged. The next release of AnyIO is some time away given how v4.10.0 was just released, so there's plenty of time. |
@agronholm I have opened a draft in my repository for fixing subprocess related bugs. I will be working it on/off depending on my part-time job's schedule. |
Changes
Fixes #959
This attempts to add winloop support to anyio. And is my second attempt. This time I not hook pre-commit so that stressful things don't happen again. I am going to be doing these steps one at a time so that I can be guided in the right direction instead of screwing up.
Checklist
If this is a user-facing code change, like a bugfix or a new feature, please ensure that
you've fulfilled the following conditions (where applicable):
tests/
) added which would fail without your patchdocs/
, in case of behavior changes or newfeatures)
docs/versionhistory.rst
).If this is a trivial change, like a typo fix or a code reformatting, then you can ignore
these instructions.
Updating the changelog
If there are no entries after the last release, use
**UNRELEASED**
as the version.If, say, your patch fixes issue #123, the entry should look like this:
If there's no issue linked, just link to your pull request instead by updating the
changelog after you've created the PR.