Skip to content
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

Merged
merged 2 commits into from Jun 2, 2020

Conversation

isidentical
Copy link
Collaborator

Cache files that weren't accessed in the 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. Resolves #120

@isidentical isidentical force-pushed the inactive-file-removal branch 5 times, most recently from 71e903a to a0126c0 Compare May 24, 2020 15:24
@davidhalter
Copy link
Owner

I might need a few days to merge this (want to review it well and I'm unfortunately not a student anymore :)).

@isidentical
Copy link
Collaborator Author

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.

@isidentical isidentical force-pushed the inactive-file-removal branch 3 times, most recently from 676000a to 3dcf75c Compare May 24, 2020 15:48
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.
@isidentical
Copy link
Collaborator Author

All green

parso/cache.py Outdated Show resolved Hide resolved
parso/cache.py Outdated Show resolved Hide resolved
parso/cache.py Outdated Show resolved Hide resolved
@davidhalter
Copy link
Owner

Great work altogether. I just think we should be a bit more lenient with errors.

@isidentical
Copy link
Collaborator Author

Thanks for the review @davidhalter, I'll try to look these next week

@isidentical isidentical force-pushed the inactive-file-removal branch 2 times, most recently from 6c917cb to 2356d56 Compare June 1, 2020 13:44
parso/cache.py Outdated
cache_path=None,
inactivity_threshold=_CACHED_FILE_MAXIMUM_SURVIVAL,
):
cache_path = _get_default_cache_path()
Copy link
Owner

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.

@davidhalter
Copy link
Owner

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.

@isidentical
Copy link
Collaborator Author

isidentical commented Jun 1, 2020

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...

@davidhalter
Copy link
Owner

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.

@davidhalter
Copy link
Owner

(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.

@isidentical isidentical merged commit fe24f0d into davidhalter:master Jun 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Parso Cache Garbage Collection
2 participants