-
Notifications
You must be signed in to change notification settings - Fork 9
option to pickle testcase lock files #65
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like where this is headed! Any measurements on impact to performance for pickle vs JSON?
| 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) |
There was a problem hiding this comment.
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?
| print(found) |
| 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)}" | ||
| ) |
There was a problem hiding this comment.
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?
| except json.JSONDecodeError: | ||
| logger.warning(f"Unable to load {file}!") | ||
| continue | ||
| state = serialize.load(file) |
There was a problem hiding this comment.
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?
pickle test case lock files. it could be faster?