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

Fallback to blocking subprocess.run if using SelectorEventLoop type #40

Merged
merged 1 commit into from
Dec 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading