Skip to content
Open
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
1 change: 1 addition & 0 deletions src/_canary/finder.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ def discover(self, pedantic: bool = True) -> list[AbstractTestGenerator]:
generators: set[AbstractTestGenerator] = set()
for root, paths in self.roots.items():
found, e = config.pluginmanager.hook.canary_discover_generators(root=root, paths=paths)
print(found)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Leftover from development? Otherwise should it be logged?

Suggested change
print(found)

generators.update(found)
errors += e
logger.debug(f"Found {len(found)} test files in {root}")
Expand Down
20 changes: 7 additions & 13 deletions src/_canary/session.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
from .third_party.lock import LockError
from .util import cpu_count
from .util import logging
from .util import serialize
from .util.filesystem import find_work_tree
from .util.filesystem import force_remove
from .util.filesystem import mkdirp
Expand Down Expand Up @@ -283,9 +284,9 @@ def dump_testcases(self) -> None:
[case.save() for case in self.cases]
else:
cpus = cpu_count()
args = ((case.getstate(), case.lockfile) for case in self.cases)
args = ((case.lockfile, case.getstate()) for case in self.cases)
pool = multiprocessing.Pool(cpus)
pool.imap_unordered(json_dump, args, chunksize=len(self.cases) // cpus or 1)
pool.imap_unordered(dump_obj, args, chunksize=len(self.cases) // cpus or 1)
pool.close()
pool.join()
index = {case.id: [dep.id for dep in case.dependencies] for case in self.cases}
Expand Down Expand Up @@ -326,12 +327,7 @@ def load_testcases(self, ids: list[str] | None = None) -> list[TestCase]:
continue
# see TestCase.lockfile for file pattern
file = self.db.join_path("cases", id[:2], id[2:], TestCase._lockfile)
with self.db.open(file) as fh:
try:
state = json.load(fh)
except json.JSONDecodeError:
logger.warning(f"Unable to load {file}!")
continue
state = serialize.load(file)
Copy link
Collaborator

Choose a reason for hiding this comment

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

The previous implies that db might be a database of some sort that provides a (potentially locked/threadsafe) file handle. Should we consider passing the open method to serialize.load, or am I thinking too far ahead given that I don't think that behavior is currently implemented by db.open anyhow?

state["properties"]["work_tree"] = self.work_tree
for i, dep_state in enumerate(state["properties"]["dependencies"]):
# assign dependencies from existing dependencies
Expand Down Expand Up @@ -677,11 +673,9 @@ def handle_signal(sig, frame):
raise SystemExit(sig)


def json_dump(args):
data, filename = args
mkdirp(os.path.dirname(filename))
with open(filename, "w") as fh:
json.dump(data, fh, indent=2)
def dump_obj(args):
filename, obj = args
serialize.dump(filename, obj)


def prefix_bits(byte_array: bytes, bits: int) -> int:
Expand Down
23 changes: 4 additions & 19 deletions src/_canary/testcase.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,7 @@
from .status import Status
from .util import filesystem as fs
from .util import logging
from .util._json import safeload
from .util._json import safesave
from .util import serialize
from .util.compression import compress_str
from .util.executable import Executable
from .util.filesystem import copyfile
Expand Down Expand Up @@ -1419,13 +1418,13 @@ def copy_sources_to_workdir(self) -> None:

def save(self):
lockfile = self.lockfile
safesave(lockfile, self.getstate())
serialize.dump(lockfile, self.getstate())
file = os.path.join(self.working_directory, self._lockfile)
mkdirp(os.path.dirname(file))
fs.force_symlink(lockfile, file)

def _load_lockfile(self) -> dict[str, Any]:
return safeload(self.lockfile)
return serialize.load(self.lockfile)

def refresh(self, propagate: bool = True) -> None:
try:
Expand Down Expand Up @@ -1612,19 +1611,6 @@ def teardown(self) -> None:
else:
fs.force_remove(file)

def dump(self, fname: str | IO[Any]) -> None:
file: IO[Any]
own_fh = False
if isinstance(fname, str):
file = open(fname, "w")
own_fh = True
else:
file = fname
state = self.getstate()
json.dump(state, file, indent=2)
if own_fh:
file.close()

def getstate(self) -> dict[str, Any]:
"""Return a serializable dictionary from which the test case can be later loaded"""
state: dict[str, Any] = {"type": self.__class__.__name__}
Expand Down Expand Up @@ -1924,8 +1910,7 @@ def from_state(state: dict[str, Any]) -> TestCase | TestMultiCase:


def from_lockfile(lockfile: str) -> TestCase | TestMultiCase:
with open(lockfile) as fh:
state = json.load(fh)
state = serialize.load(lockfile)
return from_state(state)


Expand Down
7 changes: 3 additions & 4 deletions src/_canary/testinstance.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@
from .testcase import TestCase
from .testcase import TestMultiCase
from .testcase import from_lockfile as testcase_from_lockfile
from .util._json import safeload
from .util._json import safesave
from .util import serialize

key_type = tuple[str, ...] | str
index_type = tuple[int, ...] | int
Expand Down Expand Up @@ -271,10 +270,10 @@ def gpus(self) -> int:
return len(self.gpu_ids)

def set_attribute(self, **kwargs: Any) -> None:
state = safeload(self.lockfile)
state = serialize.load(self.lockfile)
self.attributes.update(kwargs)
state["properties"].setdefault("instance_attributes").update(self.attributes)
safesave(self.lockfile, state)
serialize.dump(self.lockfile, state)

def get_dependency(self, **params: Any) -> "TestInstance | None":
for dep in self.dependencies:
Expand Down
41 changes: 0 additions & 41 deletions src/_canary/util/_json.py

This file was deleted.

51 changes: 51 additions & 0 deletions src/_canary/util/serialize.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import json
import pickle # nosec B403
import time
from pathlib import Path
from typing import Any

from .string import pluralize

PICKLE = 0
JSON = 1

protocol = JSON


def dump(file: str | Path, obj: Any) -> None:
file = Path(file)
file.parent.mkdir(parents=True, exist_ok=True)
tmp = file.with_suffix(".tmp")
if protocol == JSON:
with open(tmp, "w") as fh:
json.dump(obj, fh, indent=2)
else:
with open(tmp, "wb") as fh:
pickle.dump(obj, fh) # nosec B301
tmp.replace(file)


def load(file: str | Path, attempts: int = 8) -> Any:
file = Path(file)
delay = 0.5
attempt = 0
while attempt <= attempts:
# Guard against race condition when multiple batches are running at once
attempt += 1
try:
if protocol == JSON:
with open(file, "r") as fh:
return json.load(fh)
else:
with open(file, "rb") as fh:
return pickle.load(fh) # nosec B301
except Exception:
time.sleep(delay)
delay *= 2
raise FailedToLoadError(
f"Failed to load {file.name} after {attempts} {pluralize('attempt', attempts)}"
)
Comment on lines +30 to +47
Copy link
Collaborator

Choose a reason for hiding this comment

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

I feel like this retry pattern is common. Should we establish a retry decorator in utils and use that throughout?



class FailedToLoadError(Exception):
pass
Loading