Skip to content

Commit

Permalink
Merge pull request #40 from renan-r-santos/fix-windows-selector-loop
Browse files Browse the repository at this point in the history
Fallback to blocking `subprocess.run` if using SelectorEventLoop type
  • Loading branch information
renan-r-santos authored Dec 23, 2024
2 parents f34c6e8 + 625d1d0 commit cc8ed91
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 30 deletions.
36 changes: 22 additions & 14 deletions src/pixi_kernel/pixi.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@

import logging
import shutil
from asyncio import create_subprocess_exec
from asyncio.subprocess import PIPE, Process
import subprocess
from asyncio import SelectorEventLoop, create_subprocess_exec, get_event_loop
from asyncio.subprocess import PIPE
from pathlib import Path
from typing import Any, Optional

Expand Down Expand Up @@ -32,11 +33,18 @@ class Project(BaseModel):
manifest_path: str


async def subprocess_exec(program: str, *args: str, **kwargs: Any) -> tuple[Process, str, str]:
process = await create_subprocess_exec(program, *args, stdout=PIPE, stderr=PIPE, **kwargs)
stdout_bytes, stderr_bytes = await process.communicate()
stdout, stderr = stdout_bytes.decode("utf-8"), stderr_bytes.decode("utf-8")
return process, stdout, stderr
async def subprocess_exec(program: str, *args: str, **kwargs: Any) -> tuple[int, str, str]:
# The SelectorEventLoop does not support asyncio.subprocess
# https://github.com/renan-r-santos/pixi-kernel/issues/39
if isinstance(get_event_loop(), SelectorEventLoop):
result = subprocess.run([program, *args], capture_output=True, text=True, **kwargs) # noqa: ASYNC221
return result.returncode, result.stdout, result.stderr
else:
process = await create_subprocess_exec(program, *args, stdout=PIPE, stderr=PIPE, **kwargs)
stdout_bytes, stderr_bytes = await process.communicate()
assert process.returncode is not None
stdout, stderr = stdout_bytes.decode("utf-8"), stderr_bytes.decode("utf-8")
return process.returncode, stdout, stderr


async def ensure_readiness(
Expand Down Expand Up @@ -69,8 +77,8 @@ async def ensure_readiness(
raise RuntimeError(PIXI_NOT_FOUND.format(kernel_name=kernel_name))

# Ensure a supported Pixi version is installed
process, stdout, stderr = await subprocess_exec("pixi", "--version", env=env)
if process.returncode != 0 or not stdout.startswith("pixi "):
returncode, stdout, stderr = await subprocess_exec("pixi", "--version", env=env)
if returncode != 0 or not stdout.startswith("pixi "):
raise RuntimeError(PIXI_VERSION_ERROR.format(kernel_name=kernel_name))

# Parse Pixi version and check it against the minimum required version
Expand All @@ -85,11 +93,11 @@ async def ensure_readiness(
)

# Ensure there is a Pixi project in the current working directory or any of its parents
process, stdout, stderr = await subprocess_exec("pixi", "info", "--json", cwd=cwd, env=env)
returncode, stdout, stderr = await subprocess_exec("pixi", "info", "--json", cwd=cwd, env=env)

logger.info(f"pixi info stderr: {stderr}")
logger.info(f"pixi info stdout: {stdout}")
if process.returncode != 0:
if returncode != 0:
raise RuntimeError(f"Failed to run 'pixi info': {stderr}")

try:
Expand All @@ -102,7 +110,7 @@ async def ensure_readiness(
if pixi_info.project is None:
# Attempt to get a good error message by running `pixi project version get`. Maybe there's
# a typo in the toml file (parsing error) or there is no project at all.
process, stdout, stderr = await subprocess_exec(
returncode, stdout, stderr = await subprocess_exec(
"pixi",
"project",
"version",
Expand Down Expand Up @@ -131,8 +139,8 @@ async def ensure_readiness(
)

# Make sure the environment can be solved and is up-to-date
process, stdout, stderr = await subprocess_exec("pixi", "install", cwd=cwd, env=env)
if process.returncode != 0:
returncode, stdout, stderr = await subprocess_exec("pixi", "install", cwd=cwd, env=env)
if returncode != 0:
raise RuntimeError(f"Failed to run 'pixi install': {stderr}")

return default_environment
2 changes: 1 addition & 1 deletion src/pixi_kernel/provisioner.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ async def pre_launch(self, **kwargs: Any) -> dict[str, Any]:
logger.info(f"JupyterLab provided this value for cwd: {kwargs.get('cwd', None)}")
logger.info(f"The current working directory is {cwd}")

env: dict[str, str] = kwargs.get("env", os.environ)
env: dict[str, str] = kwargs.get("env", os.environ.copy())
pixi_environment = await ensure_readiness(
cwd=cwd.resolve(),
env=env,
Expand Down
35 changes: 22 additions & 13 deletions tests/unit/conftest.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,23 @@
import asyncio
import json
from dataclasses import dataclass
import platform

import pytest

import pixi_kernel.pixi


@dataclass
class MockProcessResult:
returncode: int
if platform.system() == "Windows":
# Test both event loop policies on Windows
# https://github.com/renan-r-santos/pixi-kernel/issues/39
@pytest.fixture(
scope="session",
params=(
asyncio.WindowsSelectorEventLoopPolicy(),
asyncio.WindowsProactorEventLoopPolicy(),
),
)
def event_loop_policy(request: pytest.FixtureRequest):
return request.param


@pytest.fixture
Expand All @@ -22,7 +31,7 @@ def _patch_pixi_version_exit_code(monkeypatch: pytest.MonkeyPatch):

async def mock_subprocess_exec(cmd, *args, **kwargs):
if cmd == "pixi" and args == ("--version",):
return MockProcessResult(1), "", ""
return 1, "", ""
else:
return await orig_subprocess_exec(cmd, *args, **kwargs)

Expand All @@ -35,7 +44,7 @@ def _patch_pixi_version_stdout(monkeypatch: pytest.MonkeyPatch):

async def mock_subprocess_exec(cmd, *args, **kwargs):
if cmd == "pixi" and args == ("--version",):
return MockProcessResult(0), "wrong output", ""
return 0, "wrong output", ""
else:
return await orig_subprocess_exec(cmd, *args, **kwargs)

Expand All @@ -48,12 +57,12 @@ def _patch_pixi_version(monkeypatch: pytest.MonkeyPatch):

async def mock_subprocess_exec(cmd, *args, **kwargs):
if cmd == "pixi" and args == ("--version",):
process, stdout, stderr = await orig_subprocess_exec(cmd, *args, **kwargs)
assert process.returncode == 0
returncode, stdout, stderr = await orig_subprocess_exec(cmd, *args, **kwargs)
assert returncode == 0
assert stdout.startswith("pixi ")

stdout = "pixi 0.15.0\n"
return process, stdout, stderr
return returncode, stdout, stderr
else:
return await orig_subprocess_exec(cmd, *args, **kwargs)

Expand All @@ -66,7 +75,7 @@ def _patch_pixi_info_exit_code(monkeypatch: pytest.MonkeyPatch):

async def mock_subprocess_exec(cmd, *args, **kwargs):
if cmd == "pixi" and args == ("info", "--json"):
return MockProcessResult(1), "", "error"
return 1, "", "error"
else:
return await orig_subprocess_exec(cmd, *args, **kwargs)

Expand All @@ -79,7 +88,7 @@ def _patch_pixi_info_stdout(monkeypatch: pytest.MonkeyPatch):

async def mock_subprocess_exec(cmd, *args, **kwargs):
if cmd == "pixi" and args == ("info", "--json"):
return MockProcessResult(0), "not JSON", ""
return 0, "not JSON", ""
else:
return await orig_subprocess_exec(cmd, *args, **kwargs)

Expand All @@ -93,7 +102,7 @@ def _patch_pixi_info_no_default_env(monkeypatch: pytest.MonkeyPatch):
async def mock_subprocess_exec(cmd, *args, **kwargs):
if cmd == "pixi" and args == ("info", "--json"):
return (
MockProcessResult(0),
0,
json.dumps(
{
"project_info": {"manifest_path": "/"},
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/test_pixi.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,8 @@ async def test_pyproject_project():
@pytest.fixture
async def env_for_pixi_in_pixi():
cwd = data_dir / "pixi_in_pixi"
process, stdout, stderr = await subprocess_exec("pixi", "run", "printenv", cwd=cwd)
assert process.returncode == 0, stderr
returncode, stdout, stderr = await subprocess_exec("pixi", "run", "printenv", cwd=cwd)
assert returncode == 0, stderr

# Update the current environment where the tests are running to merge all the env vars returned
# by `pixi run env`
Expand Down

0 comments on commit cc8ed91

Please sign in to comment.