Skip to content

Commit

Permalink
Merge pull request #65 from fal-ai/batuhan/fea-717-allow-force-creati…
Browse files Browse the repository at this point in the history
…on-of-environments-in

feat: introduce create=True flag for force creation
  • Loading branch information
isidentical authored Jan 5, 2023
2 parents 4eb6571 + 40833cf commit a583d29
Show file tree
Hide file tree
Showing 11 changed files with 84 additions and 43 deletions.
5 changes: 3 additions & 2 deletions src/isolate/backends/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,10 @@ def key(self) -> str:
and identification purposes."""
raise NotImplementedError

def create(self) -> ConnectionKeyType:
def create(self, *, force: bool = False) -> ConnectionKeyType:
"""Setup the given environment, and return all the information needed
for establishing a connection to it."""
for establishing a connection to it. If `force` flag is set, then even
if the environment is cached; it will be tried to be re-built."""
raise NotImplementedError

def destroy(self, connection_key: ConnectionKeyType) -> None:
Expand Down
40 changes: 21 additions & 19 deletions src/isolate/backends/conda.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@
import shutil
import subprocess
import tempfile
import yaml
from dataclasses import dataclass, field
from pathlib import Path
from typing import Any, ClassVar, Dict, List, Optional

import yaml

from isolate.backends import BaseEnvironment, EnvironmentCreationError
from isolate.backends.common import active_python, logged_io, sha256_digest_of
from isolate.backends.settings import DEFAULT_SETTINGS, IsolateSettings
Expand Down Expand Up @@ -43,11 +44,13 @@ def from_config(
config: Dict[str, Any],
settings: IsolateSettings = DEFAULT_SETTINGS,
) -> BaseEnvironment:
if config.get('env_dict') and config.get('env_yml_str'):
raise EnvironmentCreationError("Either env_dict or env_yml_str can be provided, not both!")
if config.get('env_yml_str'):
config['env_dict'] = yaml.safe_load(config['env_yml_str'])
del config['env_yml_str']
if config.get("env_dict") and config.get("env_yml_str"):
raise EnvironmentCreationError(
"Either env_dict or env_yml_str can be provided, not both!"
)
if config.get("env_yml_str"):
config["env_dict"] = yaml.safe_load(config["env_yml_str"])
del config["env_yml_str"]
environment = cls(**config)
environment.apply_settings(settings)
return environment
Expand All @@ -60,7 +63,7 @@ def key(self) -> str:

def _compute_dependencies(self) -> List[Any]:
if self.env_dict:
user_dependencies = self.env_dict.get('dependencies', []).copy()
user_dependencies = self.env_dict.get("dependencies", []).copy()
else:
user_dependencies = self.packages.copy()
for raw_requirement in user_dependencies:
Expand Down Expand Up @@ -97,28 +100,25 @@ def _compute_dependencies(self) -> List[Any]:
user_dependencies.append(f"python={target_python}")
return user_dependencies

def create(self) -> Path:
def create(self, *, force: bool = False) -> Path:
env_path = self.settings.cache_dir_for(self)
with self.settings.cache_lock_for(env_path):
if env_path.exists():
if env_path.exists() and not force:
return env_path

if self.env_dict:
self.env_dict['dependencies'] = self._compute_dependencies()
with tempfile.NamedTemporaryFile(mode='w', suffix='.yml') as tf:
self.env_dict["dependencies"] = self._compute_dependencies()
with tempfile.NamedTemporaryFile(mode="w", suffix=".yml") as tf:
yaml.dump(self.env_dict, tf)
tf.flush()
try:
self._run_conda(
"env",
"create",
"-f",
tf.name,
"--prefix",
env_path
"env", "create", "-f", tf.name, "--prefix", env_path
)
except subprocess.SubprocessError as exc:
raise EnvironmentCreationError("Failure during 'conda create'") from exc
raise EnvironmentCreationError(
"Failure during 'conda create'"
) from exc

else:
# Since our agent needs Python to be installed (at very least)
Expand All @@ -139,7 +139,9 @@ def create(self) -> Path:
*dependencies,
)
except subprocess.SubprocessError as exc:
raise EnvironmentCreationError("Failure during 'conda create'") from exc
raise EnvironmentCreationError(
"Failure during 'conda create'"
) from exc

self.log(f"New environment cached at '{env_path}'")
return env_path
Expand Down
6 changes: 5 additions & 1 deletion src/isolate/backends/local.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,11 @@ def from_config(
def key(self) -> str:
return sha256_digest_of(sys.exec_prefix)

def create(self) -> Path:
def create(self, *, force: bool = False) -> Path:
if force is True:
raise NotImplementedError(
"LocalPythonEnvironment cannot be forcibly created"
)
return Path(sys.exec_prefix)

def destroy(self, connection_key: Path) -> None:
Expand Down
4 changes: 2 additions & 2 deletions src/isolate/backends/pyenv.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ def from_config(
def key(self) -> str:
return os.path.join("versions", self.python_version)

def create(self) -> Path:
def create(self, *, force: bool = False) -> Path:
pyenv = _get_pyenv_executable()
env_path = self.settings.cache_dir_for(self)
with self.settings.cache_lock_for(env_path):
Expand All @@ -47,7 +47,7 @@ def create(self) -> Path:
# [0]: https://github.com/pyenv/pyenv#locating-pyenv-provided-python-installations
pyenv_root = env_path.parent.parent
prefix = self._try_get_prefix(pyenv, pyenv_root)
if prefix is None:
if prefix is None or force:
self._install_python(pyenv, pyenv_root)
prefix = self._try_get_prefix(pyenv, pyenv_root)
if not prefix:
Expand Down
12 changes: 11 additions & 1 deletion src/isolate/backends/remote.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from __future__ import annotations

import copy
import json
from dataclasses import dataclass
from typing import Any, ClassVar, Dict, List, Optional
Expand Down Expand Up @@ -47,15 +48,24 @@ def key(self) -> str:
json.dumps(self.target_environments),
)

def create(self) -> List[EnvironmentDefinition]:
def create(self, *, force: bool = False) -> List[EnvironmentDefinition]:
if force is True:
raise NotImplementedError(
"Only individual environments can be forcibly created, please set them up"
" manually by using the 'force_create' flag on the environment definition."
)

envs = []
for env in self.target_environments:
if not env.get("kind") or not env.get("configuration"):
raise RuntimeError(f"`kind` or `configuration` key missing in: {env}")
configuration = copy.deepcopy(env["configuration"])
force_create = configuration.pop("force_create", False)
envs.append(
EnvironmentDefinition(
kind=env["kind"],
configuration=interface.to_struct(env["configuration"]),
force=force_create,
)
)
return envs
Expand Down
4 changes: 2 additions & 2 deletions src/isolate/backends/virtualenv.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,12 +119,12 @@ def _decide_python(self) -> str:
else:
return self._install_python_through_pyenv()

def create(self) -> Path:
def create(self, *, force: bool = False) -> Path:
from virtualenv import cli_run

venv_path = self.settings.cache_dir_for(self)
with self.settings.cache_lock_for(venv_path):
if venv_path.exists():
if venv_path.exists() and not force:
return venv_path

self.log(f"Creating the environment at '{venv_path}'")
Expand Down
2 changes: 2 additions & 0 deletions src/isolate/server/definitions/server.proto
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,6 @@ message EnvironmentDefinition {
string kind = 1;
// A free-form definition of environment properties.
google.protobuf.Struct configuration = 2;
// Whether to force-create this environment or not.
bool force = 3;
}
8 changes: 4 additions & 4 deletions src/isolate/server/definitions/server_pb2.py

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 5 additions & 1 deletion src/isolate/server/definitions/server_pb2.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -57,24 +57,28 @@ class EnvironmentDefinition(google.protobuf.message.Message):

KIND_FIELD_NUMBER: builtins.int
CONFIGURATION_FIELD_NUMBER: builtins.int
FORCE_FIELD_NUMBER: builtins.int
kind: builtins.str
"""Kind of the isolate environment."""
@property
def configuration(self) -> google.protobuf.struct_pb2.Struct:
"""A free-form definition of environment properties."""
force: builtins.bool
"""Whether to force-create this environment or not."""
def __init__(
self,
*,
kind: builtins.str = ...,
configuration: google.protobuf.struct_pb2.Struct | None = ...,
force: builtins.bool = ...,
) -> None: ...
def HasField(
self, field_name: typing_extensions.Literal["configuration", b"configuration"]
) -> builtins.bool: ...
def ClearField(
self,
field_name: typing_extensions.Literal[
"configuration", b"configuration", "kind", b"kind"
"configuration", b"configuration", "force", b"force", "kind", b"kind"
],
) -> None: ...

Expand Down
16 changes: 9 additions & 7 deletions src/isolate/server/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def Run(
environments = []
for env in request.environments:
try:
environments.append(from_grpc(env))
environments.append((env.force, from_grpc(env)))
except ValueError:
return self.abort_with_msg(
f"Unknown environment kind: {env.kind}",
Expand All @@ -76,10 +76,10 @@ def Run(
serialization_method=request.function.method,
)

for environment in environments:
for _, environment in environments:
environment.apply_settings(run_settings)

primary_environment = environments[0]
_, primary_environment = environments[0]

if AGENT_REQUIREMENTS:
python_version = getattr(
Expand All @@ -90,7 +90,7 @@ def Run(
python_version=python_version,
)
agent_environ.apply_settings(run_settings)
environments.insert(1, agent_environ)
environments.insert(1, (False, agent_environ))

extra_inheritance_paths = []
if INHERIT_FROM_LOCAL:
Expand All @@ -99,8 +99,10 @@ def Run(

with ThreadPoolExecutor(max_workers=1) as local_pool:
environment_paths = []
for environment in environments:
future = local_pool.submit(environment.create)
for should_force_create, environment in environments:
future = local_pool.submit(
environment.create, force=should_force_create
)
yield from self.watch_queue_until_completed(messages, future.done)
try:
# Assuming that the iterator above only stops yielding once
Expand All @@ -115,7 +117,7 @@ def Run(

primary_path, *inheritance_paths = environment_paths
inheritance_paths.extend(extra_inheritance_paths)
primary_environment = environments[0]
_, primary_environment = environments[0]

with LocalPythonGRPC(
primary_environment,
Expand Down
24 changes: 20 additions & 4 deletions tests/test_backends.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,23 @@ def test_failure_during_environment_creation_cache(self, tmp_path, monkeypatch):
assert environment.exists()
assert self.get_example_version(environment, connection_key) == "0.6.0"

def test_forced_environment_creation(self, tmp_path, monkeypatch):
environment = self.get_project_environment(tmp_path, "new-example-project")
environment.create()

# Environment exists at this point, and if we try to create it again
# (even if it is not possible to do so, e.g. the directory is read-only)
# the create() call will succeed (since it is cached).
assert environment.exists()
with self.fail_active_creation(monkeypatch):
environment.create() # No problem!

# But if we force the creation of the environment, then it should
# fail since the directory is read-only.
with pytest.raises(EnvironmentCreationError):
with self.fail_active_creation(monkeypatch):
environment.create(force=True)

def test_invalid_project_building(self, tmp_path, monkeypatch):
environment = self.get_project_environment(tmp_path, "invalid-project")
with pytest.raises(EnvironmentCreationError):
Expand Down Expand Up @@ -384,11 +401,9 @@ class TestConda(GenericEnvironmentTests):
"env_dict": {
"name": "test",
"channels": "defaults",
"dependencies": [
{"pip": ["pyjokes==0.5.0"]}
]
"dependencies": [{"pip": ["pyjokes==0.5.0"]}],
}
}
},
}
creation_entry_point = ("subprocess.check_call", subprocess.SubprocessError)

Expand Down Expand Up @@ -440,6 +455,7 @@ def test_fail_when_user_overwrites_python(
):
environment.create()


def test_local_python_environment():
"""Since 'local' environment does not support installation of extra dependencies
unlike virtualenv/conda, we can't use the generic test suite for it."""
Expand Down

0 comments on commit a583d29

Please sign in to comment.