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

SQLite-based read-write lock #399

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

SQLite-based read-write lock #399

wants to merge 5 commits into from

Conversation

leventov
Copy link

@leventov leventov commented Feb 19, 2025

Draft for #307.

@gaborbernat @Yard1 please let me know how I should change the code from the API perspective. There are quite a bit very elaborate stuff that is hard to understand regarding thread-locals, etc.

@leventov leventov marked this pull request as draft February 19, 2025 18:24
super().__init__(lock_file, timeout, blocking)
self.procLock = threading.Lock()
self.con = sqlite3.connect(self._context.lock_file, check_same_thread=False)
# Redundant unless there are "rogue" processes that open the db
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please align comments to 120 charachters 🙇



class _SQLiteLock(BaseFileLock):
def __init__(self, lock_file: str | os.PathLike[str], timeout: float = -1, blocking: bool = True) -> None:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer

from os import PathLike

@leventov
Copy link
Author

@gaborbernat I've pushed a more "serious" attempt.

Copy link
Member

@gaborbernat gaborbernat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks better, can you add tests please?

@leventov
Copy link
Author

@gaborbernat what do you think about using SQLite's VFS interface directly, without the "dummy database" thing? https://github.com/rogerbinns/apsw provides bindings

@gaborbernat
Copy link
Member

gaborbernat commented Feb 25, 2025 via email

@leventov
Copy link
Author

leventov commented Mar 4, 2025

@gaborbernat I've added tests.

If you want to finesse API (context managers vs. parameters to acquire_read() and acquire_write() methods; are read_lock() and write_lock() methods even needed? Are instance's default timeout and blocking needed?), docs, formatting, class/method naming, etc., could you please do it yourself to your liking?

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.

2 participants