Skip to content

Conversation

@evtimovi
Copy link
Contributor

@evtimovi evtimovi commented Dec 9, 2025

This pull request creates a plugin system for what is backing the Docker implementation when the SandboxManager is docker-based. This allows us to have different plugins based on what Docker libraries we want to use and avoid having to rewrite other parts of the SandboxManager for this purpose. Here, we also abstract the aiodocker behind a LocalDocker plugin.

@meta-cla meta-cla bot added the CLA Signed This label is managed by the Meta Open Source bot. label Dec 9, 2025
pytestmark = pytest.mark.anyio

# Path to test fixtures
FIXTURES_DIR = Path(__file__).parent / "fixtures"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should use resources.files from importlib import resources, i.e., use resources.files("tests.integration.fixtures").

This returns a Path-like object on which we can then call str later on

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, I am not sure why we need to get an object like this and then cast it back to string; what is the benefit of this?

test_image: str,
docker_client_type: str,
skip_if_des_unavailable,
create_manager_config,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: ideally we can ask an LLM to add type annotations for the fixtures

Copy link
Member

@vitoralbiero vitoralbiero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stamping to unblock

@evtimovi evtimovi merged commit cb5c5b2 into main Dec 10, 2025
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants