ex0dus-0x / microkv

Minimal and persistent key-value store designed with security in mind
MIT License
32 stars 6 forks source link

Make read and write locks useful #15

Closed plainerman closed 1 year ago

plainerman commented 1 year ago

This PR would close #14. It servers as a proposal to discuss the direction we should go to solve this problem. If this is the way we want to go, I can extend it to the remaining useful functions and we can merge the PR.

ex0dus-0x commented 1 year ago

Thanks for bringing up your concerns and starting the PR! Your suggestions make sense, as locked operations certainly would require some manual sanitization/encryption on the users end. This was not prior since it wasn't something I needed for my use case (I only cared about basic primitives most of the time), but would be nice for other's.

PR looks good to me so far, be sure to lint and add test cases too :)

plainerman commented 1 year ago

Great, then I will do the same for the remaining functions, document the changes, and see that the pipeline passes.

One question still: I would say, that after write_lock the database should auto_commit (if the variable is set). Do you agree?

ex0dus-0x commented 1 year ago

That sounds good! It looks like auto_commit doesn't have much of an effect at the moment, so would definitely appreciate adding the functionality there.

plainerman commented 1 year ago

Could you look at the changes once more? I moved the commit part into the lock function, added doc-tests, and simplified the calls from the namespace class.

From my side, the changes are good to go, since the previous tests also test the new code.

ex0dus-0x commented 1 year ago

Thanks @plainerman for your contributions! The refactoring to lock acquisition and passing in a callback all make sense to me, and am also grateful for getting the auto_commit functionality to actually operate.

Appreciate all the help with this project!

plainerman commented 1 year ago

Thank you very much for merging this so quickly!