No more weird caching issues. #2
Conversation
Handles concurrency issues.
brycedrennan
force-pushed
the
better-cache
branch
from
307f46a
to
c20317a
Compare
|
I do wish I had explored that route more as that would have been easier to maintain. There is some reason to think its not possible though because of how the library is designed. I still think we should fork, as we have done in other cases because:
|
There was a problem hiding this comment.
If backing out these changes and fixing via the method I mentioned in my initial comment is not an option, please update readme for this repo to mention how/when this fork happened + additional relevant background around the changes we introduced. We should either remove or update the badges as well since they still point to the upstream version.
tldextract/utils.py
Outdated
| if not os.path.isfile(cache_path): | ||
| result = func(*args, **kwargs) | ||
| with open(cache_path, 'w') as cache_file: | ||
| json.dump(result, cache_file) | ||
|
|
||
| with open(cache_path) as cache_file: | ||
| return json.load(cache_file) |
There was a problem hiding this comment.
shouldn't the read also be protected by the lock?
|
Pleas also update permissions on this repo. I am currently seeing the following message:
|
|
good feedback. will do. |
|
@jessevogt It locks for reads now and the team has access to the repo. |
Our pull request to the parent library has been ignored. Lets deploy to our private repo.
john-kurkowski#144
This is already deployed to pypi.cu and in use now.
http://pypi.cu/root/circleup/tldextract/3.0.0.circleup