-
Notifications
You must be signed in to change notification settings - Fork 106
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
Clear inactive cache files #121
Clear inactive cache files #121
Conversation
71e903a
to
a0126c0
Compare
I might need a few days to merge this (want to review it well and I'm unfortunately not a student anymore :)). |
No problem at all, in the meanwhile, I need to make this patch working on 3.5< with my force-pushes to use this CI. |
676000a
to
3dcf75c
Compare
Cache files that wasn't accessed in last 30 days will be automatically garbage collected. This collection happens when the `save_module` is called via a lock system that would make it happen only one time per day.
3dcf75c
to
21e54c3
Compare
All green |
Great work altogether. I just think we should be a bit more lenient with errors. |
Thanks for the review @davidhalter, I'll try to look these next week |
6c917cb
to
2356d56
Compare
parso/cache.py
Outdated
cache_path=None, | ||
inactivity_threshold=_CACHED_FILE_MAXIMUM_SURVIVAL, | ||
): | ||
cache_path = _get_default_cache_path() |
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.
There should probably be an if cache_path is None
before this.
And as a minor nitpick: I actually prefer if people don't force push in pull requests. It's very hard to understand what has been changed. I can still squash if I really need to when I merge. |
I try to keep the commit log clean, like for the last force pushes I was using trying to fix a CI failure on the same commit so I didn't want to create new commits just for that. But I respect your preference, so try to avoid them. EDIT: ups, sorry for the last push, muscle memory :/ Try to improve it... |
2356d56
to
dbbaf09
Compare
As I said, it's a nitpick, feel free to do it however you want to. I feel it's obviously easier to spot the differences in PRs, but I can still work around that. I also personally think that commit logs don't matter nearly as much as people think they do. If I want to write a longer text in a commit log I usually abort and make the exact comment in code. But here opinions tend to vary a lot and I'm probably not even part of the majority. In the end all I want is readable concise error-free code. |
(and as a small addition, I like to actually have the whole history of changes. For example I think your first iteration was really interesting, because it was simply another approach, but as I said, a lot of people don't feel that way) Feel free to merge when you think you're done. |
Cache files that weren't accessed in the last 30 days will be automatically
garbage collected. This collection happens when the
save_module
is calledvia a lock system that would make it happen only one time per day. Resolves #120