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

use threading.Lock for cache maintenance functions #496

Closed
wants to merge 1 commit into from

Conversation

AhmetNSimsek
Copy link
Collaborator

Solves #489

@AhmetNSimsek AhmetNSimsek added the maintenance Not a bug or breaking issue. Code maintenance related. label Oct 25, 2023
@AhmetNSimsek AhmetNSimsek requested a review from xgui3783 October 25, 2023 12:10
@codecov-commenter
Copy link

Codecov Report

Merging #496 (269dc2d) into main (5a0e6b3) will increase coverage by 1.45%.
Report is 186 commits behind head on main.
The diff coverage is 35.86%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@            Coverage Diff             @@
##             main     #496      +/-   ##
==========================================
+ Coverage   36.81%   38.27%   +1.45%     
==========================================
  Files          61       62       +1     
  Lines        5421     5816     +395     
==========================================
+ Hits         1996     2226     +230     
- Misses       3425     3590     +165     
Files Coverage Δ
siibra/core/atlas.py 85.71% <ø> (+1.23%) ⬆️
siibra/features/connectivity/__init__.py 83.33% <100.00%> (ø)
...ibra/features/connectivity/tracing_connectivity.py 75.00% <100.00%> (ø)
siibra/features/image/__init__.py 100.00% <100.00%> (ø)
siibra/features/tabular/layerwise_cell_density.py 37.20% <ø> (+0.25%) ⬆️
siibra/locations/boundingbox.py 19.88% <ø> (ø)
siibra/configuration/configuration.py 42.35% <0.00%> (ø)
...a/features/connectivity/functional_connectivity.py 55.55% <0.00%> (ø)
siibra/livequeries/ebrains.py 25.35% <0.00%> (+0.35%) ⬆️
siibra/volumes/__init__.py 83.33% <50.00%> (-16.67%) ⬇️
... and 31 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@AhmetNSimsek AhmetNSimsek marked this pull request as draft October 26, 2023 13:10
@AhmetNSimsek
Copy link
Collaborator Author

TO-DO:
Use the following design scheme instead:

from functools import wraps

def get_lock(filepath: str):
    def outer(fn):
        @wraps(fn)
        def inner(*args, **kwargs):
            with FileLock(filepath):
                return fn(*args, **kwargs)
        return inner
    return outer


@get_lock("access_cache")
def warm_cache():
    pass

@get_lock("access_cache")
def maintenance():
    ...

@AhmetNSimsek AhmetNSimsek force-pushed the maint_cache_thread_safety branch from 269dc2d to 9458f08 Compare December 14, 2023 10:16
@AhmetNSimsek
Copy link
Collaborator Author

This is not working as intended. Using #496 (comment) is still an option but a more comprehensive maintenance of cache.py is required. Closing this PR for an an alternative including a comprehensive maintenance and filelock.

@AhmetNSimsek AhmetNSimsek deleted the maint_cache_thread_safety branch December 14, 2023 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Not a bug or breaking issue. Code maintenance related.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants