Skip to content

Commit

Permalink
Merge pull request #37 from renan-r-santos/fix-blocking-code
Browse files Browse the repository at this point in the history
Fix blocking `subprocess.run`
  • Loading branch information
renan-r-santos authored Dec 21, 2024
2 parents 59186c4 + 9c49323 commit e99b3a4
Show file tree
Hide file tree
Showing 15 changed files with 480 additions and 462 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ jobs:
fail-fast: false
matrix:
os: [ubuntu-latest, macos-latest, windows-latest]
pixi-version: ["0.30.0", "0.38.0"]
pixi-version: ["0.30.0", "0.39.3"]
runs-on: ${{ matrix.os }}
steps:
- name: Checkout repo
Expand All @@ -25,7 +25,7 @@ jobs:
run-install: false

- name: Install uv
uses: astral-sh/setup-uv@v4
uses: astral-sh/setup-uv@v5

- name: Test, lint and typecheck
run: uv run tox
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ jobs:
uses: actions/checkout@v4

- name: Install uv
uses: astral-sh/setup-uv@v4
uses: astral-sh/setup-uv@v5

- name: Build release
run: uv build
Expand Down
3 changes: 2 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ dev-dependencies = [
"jupyter-kernel-test>=0.7,<0.8",
"mypy>=1,<2",
"pytest>=8,<9",
"pytest-asyncio>=0.24,<0.25",
"pytest-asyncio>=0.25,<0.26",
"pytest-cov>=6,<7",
"ruff>=0.8,<0.9",
"tox-uv>=1,<2",
Expand Down Expand Up @@ -83,6 +83,7 @@ strict = true
[tool.pytest.ini_options]
addopts = ["--strict-config", "--strict-markers"]
asyncio_default_fixture_loop_scope = "function"
asyncio_mode = "auto"
filterwarnings = [
"ignore:Jupyter is migrating its paths to use standard platformdirs",
]
Expand Down
58 changes: 31 additions & 27 deletions src/pixi_kernel/pixi.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@

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

from pydantic import BaseModel, Field, ValidationError

Expand All @@ -31,7 +32,14 @@ class Project(BaseModel):
manifest_path: str


def ensure_readiness(
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 ensure_readiness(
*,
cwd: Path,
env: dict[str, str],
Expand Down Expand Up @@ -61,12 +69,12 @@ def ensure_readiness(
raise RuntimeError(PIXI_NOT_FOUND.format(kernel_name=kernel_name))

# Ensure a supported Pixi version is installed
result = subprocess.run(["pixi", "--version"], capture_output=True, env=env, text=True)
if result.returncode != 0 or not result.stdout.startswith("pixi "):
process, stdout, stderr = await subprocess_exec("pixi", "--version", env=env)
if process.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
pixi_version = result.stdout[len("pixi ") :].strip()
pixi_version = stdout[len("pixi ") :].strip()

major, minor, patch = map(int, pixi_version.split("."))
required_major, required_minor, required_patch = map(int, MINIMUM_PIXI_VERSION.split("."))
Expand All @@ -77,36 +85,32 @@ def ensure_readiness(
)

# Ensure there is a Pixi project in the current working directory or any of its parents
result = subprocess.run(
["pixi", "info", "--json"],
cwd=cwd,
capture_output=True,
env=env,
text=True,
)
logger.info(f"pixi info stderr: {result.stderr}")
logger.info(f"pixi info stdout: {result.stdout}")
if result.returncode != 0:
raise RuntimeError(f"Failed to run 'pixi info': {result.stderr}")
process, 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:
raise RuntimeError(f"Failed to run 'pixi info': {stderr}")

try:
pixi_info = PixiInfo.model_validate_json(result.stdout, strict=True)
pixi_info = PixiInfo.model_validate_json(stdout, strict=True)
except ValidationError as exception:
raise RuntimeError(
f"Failed to parse 'pixi info' output: {result.stdout}\n{exception}"
f"Failed to parse 'pixi info' output: {stdout}\n{exception}"
) from exception

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.
result = subprocess.run(
["pixi", "project", "version", "get"],
process, stdout, stderr = await subprocess_exec(
"pixi",
"project",
"version",
"get",
cwd=cwd,
capture_output=True,
env=env,
text=True,
)
raise RuntimeError(result.stderr)
raise RuntimeError(stderr)

# Find the default environment and check if the required kernel package is a dependency
for pixi_env in pixi_info.environments:
Expand All @@ -127,8 +131,8 @@ def ensure_readiness(
)

# Make sure the environment can be solved and is up-to-date
result = subprocess.run(["pixi", "install"], cwd=cwd, capture_output=True, env=env, text=True)
if result.returncode != 0:
raise RuntimeError(f"Failed to run 'pixi install': {result.stderr}")
process, stdout, stderr = await subprocess_exec("pixi", "install", cwd=cwd, env=env)
if process.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 @@ -35,7 +35,7 @@ async def pre_launch(self, **kwargs: Any) -> dict[str, Any]:
logger.info(f"The current working directory is {cwd}")

env: dict[str, str] = kwargs.get("env", os.environ)
pixi_environment = ensure_readiness(
pixi_environment = await ensure_readiness(
cwd=cwd.resolve(),
env=env,
required_package=required_package,
Expand Down
98 changes: 52 additions & 46 deletions tests/unit/conftest.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,15 @@
import json
import subprocess
from dataclasses import dataclass

import pytest

import pixi_kernel.pixi


@dataclass
class MockProcessResult:
returncode: int


@pytest.fixture
def _patch_path(monkeypatch: pytest.MonkeyPatch):
Expand All @@ -11,84 +18,83 @@ def _patch_path(monkeypatch: pytest.MonkeyPatch):

@pytest.fixture
def _patch_pixi_version_exit_code(monkeypatch: pytest.MonkeyPatch):
original_run = subprocess.run
orig_subprocess_exec = pixi_kernel.pixi.subprocess_exec

def mock_run(cmd, *args, **kwargs):
if cmd == ["pixi", "--version"]:
return subprocess.CompletedProcess(cmd, returncode=1, stdout="", stderr="")
async def mock_subprocess_exec(cmd, *args, **kwargs):
if cmd == "pixi" and args == ("--version",):
return MockProcessResult(1), "", ""
else:
return original_run(cmd, *args, **kwargs)
return await orig_subprocess_exec(cmd, *args, **kwargs)

monkeypatch.setattr("subprocess.run", mock_run)
monkeypatch.setattr("pixi_kernel.pixi.subprocess_exec", mock_subprocess_exec)


@pytest.fixture
def _patch_pixi_version_stdout(monkeypatch: pytest.MonkeyPatch):
original_run = subprocess.run
orig_subprocess_exec = pixi_kernel.pixi.subprocess_exec

def mock_run(cmd, *args, **kwargs):
if cmd == ["pixi", "--version"]:
return subprocess.CompletedProcess(cmd, returncode=0, stdout="wrong output", stderr="")
async def mock_subprocess_exec(cmd, *args, **kwargs):
if cmd == "pixi" and args == ("--version",):
return MockProcessResult(0), "wrong output", ""
else:
return original_run(cmd, *args, **kwargs)
return await orig_subprocess_exec(cmd, *args, **kwargs)

monkeypatch.setattr("subprocess.run", mock_run)
monkeypatch.setattr("pixi_kernel.pixi.subprocess_exec", mock_subprocess_exec)


@pytest.fixture
def _patch_pixi_version(monkeypatch: pytest.MonkeyPatch):
original_run = subprocess.run
orig_subprocess_exec = pixi_kernel.pixi.subprocess_exec

def mock_run(cmd, *args, **kwargs):
if cmd == ["pixi", "--version"]:
result = original_run(cmd, *args, **kwargs)
assert result.returncode == 0
assert result.stdout.startswith("pixi ")
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
assert stdout.startswith("pixi ")

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

monkeypatch.setattr("subprocess.run", mock_run)
monkeypatch.setattr("pixi_kernel.pixi.subprocess_exec", mock_subprocess_exec)


@pytest.fixture
def _patch_pixi_info_exit_code(monkeypatch: pytest.MonkeyPatch):
original_run = subprocess.run
orig_subprocess_exec = pixi_kernel.pixi.subprocess_exec

def mock_run(cmd, *args, **kwargs):
if cmd == ["pixi", "info", "--json"]:
return subprocess.CompletedProcess(cmd, returncode=1, stdout="", stderr="error")
async def mock_subprocess_exec(cmd, *args, **kwargs):
if cmd == "pixi" and args == ("info", "--json"):
return MockProcessResult(1), "", "error"
else:
return original_run(cmd, *args, **kwargs)
return await orig_subprocess_exec(cmd, *args, **kwargs)

monkeypatch.setattr("subprocess.run", mock_run)
monkeypatch.setattr("pixi_kernel.pixi.subprocess_exec", mock_subprocess_exec)


@pytest.fixture
def _patch_pixi_info_stdout(monkeypatch: pytest.MonkeyPatch):
original_run = subprocess.run
orig_subprocess_exec = pixi_kernel.pixi.subprocess_exec

def mock_run(cmd, *args, **kwargs):
if cmd == ["pixi", "info", "--json"]:
return subprocess.CompletedProcess(cmd, returncode=0, stdout="not JSON", stderr="")
async def mock_subprocess_exec(cmd, *args, **kwargs):
if cmd == "pixi" and args == ("info", "--json"):
return MockProcessResult(0), "not JSON", ""
else:
return original_run(cmd, *args, **kwargs)
return await orig_subprocess_exec(cmd, *args, **kwargs)

monkeypatch.setattr("subprocess.run", mock_run)
monkeypatch.setattr("pixi_kernel.pixi.subprocess_exec", mock_subprocess_exec)


@pytest.fixture
def _patch_pixi_info_no_default_env(monkeypatch: pytest.MonkeyPatch):
original_run = subprocess.run

def mock_run(cmd, *args, **kwargs):
if cmd == ["pixi", "info", "--json"]:
return subprocess.CompletedProcess(
cmd,
returncode=0,
stdout=json.dumps(
orig_subprocess_exec = pixi_kernel.pixi.subprocess_exec

async def mock_subprocess_exec(cmd, *args, **kwargs):
if cmd == "pixi" and args == ("info", "--json"):
return (
MockProcessResult(0),
json.dumps(
{
"project_info": {"manifest_path": "/"},
"environments_info": [
Expand All @@ -101,9 +107,9 @@ def mock_run(cmd, *args, **kwargs):
],
}
),
stderr="",
"",
)
else:
return original_run(cmd, *args, **kwargs)
return await orig_subprocess_exec(cmd, *args, **kwargs)

monkeypatch.setattr("subprocess.run", mock_run)
monkeypatch.setattr("pixi_kernel.pixi.subprocess_exec", mock_subprocess_exec)
2 changes: 1 addition & 1 deletion tests/unit/data/bad_pixi_toml/pixi.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@
name = "pixi_project"
version = "0.1.0"
channels = ["conda-forge"]
platforms = ["linux-64"]
platforms = ["linux-64", "linux-aarch64", "osx-64", "osx-arm64", "win-64"]

[dependencies_typo]
2 changes: 1 addition & 1 deletion tests/unit/data/missing_ipykernel/pixi.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
name = "missing_ipykernel"
version = "0.1.0"
channels = ["conda-forge"]
platforms = ["linux-64"]
platforms = ["linux-64", "linux-aarch64", "osx-64", "osx-arm64", "win-64"]

[dependencies]
python = "*"
2 changes: 1 addition & 1 deletion tests/unit/data/non_existing_dependency/pixi.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
name = "pixi_project"
version = "0.1.0"
channels = ["conda-forge"]
platforms = ["linux-64", "osx-64", "win-64"]
platforms = ["linux-64", "linux-aarch64", "osx-64", "osx-arm64", "win-64"]

[dependencies]
python = "4.*"
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/data/pixi_in_pixi/good_project/pixi.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
name = "pixi_project"
version = "0.1.0"
channels = ["conda-forge"]
platforms = ["linux-64", "osx-64", "win-64"]
platforms = ["linux-64", "linux-aarch64", "osx-64", "osx-arm64", "win-64"]

[dependencies]
python = "*"
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/data/pixi_in_pixi/pixi.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@
name = "empty_project"
version = "0.1.0"
channels = ["conda-forge"]
platforms = ["linux-64", "osx-64", "win-64"]
platforms = ["linux-64", "linux-aarch64", "osx-64", "osx-arm64", "win-64"]

[dependencies]
2 changes: 1 addition & 1 deletion tests/unit/data/pixi_project/pixi.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
name = "pixi_project"
version = "0.1.0"
channels = ["conda-forge"]
platforms = ["linux-64", "osx-64", "win-64"]
platforms = ["linux-64", "linux-aarch64", "osx-64", "osx-arm64", "win-64"]

[dependencies]
python = "*"
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/data/pyproject_project/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ version = "0.1.0"

[tool.pixi.project]
channels = ["conda-forge"]
platforms = ["linux-64", "osx-64", "win-64"]
platforms = ["linux-64", "linux-aarch64", "osx-64", "osx-arm64", "win-64"]

[tool.pixi.pypi-dependencies]
ipykernel = "*"
Loading

0 comments on commit e99b3a4

Please sign in to comment.