-
Notifications
You must be signed in to change notification settings - Fork 5
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
Lock cache file with python #168
base: master
Are you sure you want to change the base?
Conversation
lib/id3c/cli/command/geocode.py
Outdated
@@ -31,10 +31,10 @@ | |||
from smartystreets_python_sdk.us_street import Lookup | |||
from smartystreets_python_sdk.us_extract import Lookup as ExtractLookup | |||
from id3c.cli import cli | |||
from id3c.cli.command import pickled_cache |
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.
This edit can be dropped from this commit.
06b84f8
to
ea9d469
Compare
ea9d469
to
77eb693
Compare
@@ -125,7 +126,9 @@ def pickled_cache(filename: str = None) -> TTLCache: | |||
LOG.info(f"Loading cache from «{filename}»") | |||
try: | |||
with open(filename, "rb") as file: | |||
lock_file(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.
I can't comment on the implementation of lock_file()
yet, but I think the usage here isn't doing what you expect because it's not being called as a context manager. It should be called as:
with lock_file(file):
…
I think ultimately we may want to switch the geocoding caching from our in-house solution to diskcache, which I used for the Husky Musher caching. When we first added the caching, I didn't know diskcache existed and, unlike cachetools, didn't uncover it during due diligence. @joverlee521 asked in Slack:
and I thought it'd be useful to copy my answer here as well:
|
Lock the cache file in python rather than relying on the user knowing to lock at runtime with shell's
flock
, etc.Open questions:
How do we want to handle cache collision?
Right now if I run two REDCap DET ETL processes operating on the same cache, I sometimes get
Perhaps this means my implementation isn't working as expected?
Which command(s) do we want to use when creating a lock?