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

Implement exclude filters #837

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

muturgan
Copy link

Closes #5

@vbrandl
Copy link
Owner

vbrandl commented Oct 17, 2024

Using multiple cache files, for each set of excluded files is way simpler then changing the cache format as I had it in mind. Good idea.

I will test this soon.

Thinking about it: having multiple cache files breaks the delete_repo_and_cache function. To fix this, we would have to use something like glob, but deleting cache/github.com/username/reponame*.json would also delete caches for every other repository of username which name starts with reponame.

I think the cleanest and correct way would be to add another layer into the cache format and make it a HashMap<Branch, HashMap<Excludes, CacheEntry>>. And while at it, there should be some kind of locking mechanism when accessing the cache. Currently two parallel request might cause conflicting cache writes, causing a corrupt cache.

@muturgan
Copy link
Author

Well, need think about it

@muturgan
Copy link
Author

I think delete all files by pattern is what we need. If a hoc score has changed on all repository - this has changed (most likely) for a some subset of files.
But as you say branch is important, need add it to a cache file path.

@muturgan
Copy link
Author

May access to a cache via rwlock

@muturgan
Copy link
Author

@vbrandl do you know a cache size on prod? Can we store it in memory?

@vbrandl
Copy link
Owner

vbrandl commented Oct 23, 2024

Using an in-memory cache would also be fine, I think. Could you first describe, how you plan to implement said cache?

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.

Implement exclude filters
2 participants