search

mbr / simplekv

A simple key-value store for binary data.
http://simplekv.readthedocs.io
MIT License
152 stars 50 forks source link

Proposal for closable stores #115

Open timostrunk opened 2 years ago

timostrunk commented 2 years ago

Motivation

simplekv does not have a possibility to close a store, if opening it requires opening e.g. file descriptors or ports. This can for example be observed in the Azure blobstore unit tests, when turning on ResourceWarnings, which generates 1253 warnings and 10000 lines noting that ports were not closed:

pytest -Wonce tests/test_azure_store.py
============================================================================================== test session starts ==============================================================================================
platform linux -- Python 3.10.6, pytest-7.1.3, pluggy-1.0.0
rootdir: /home/me/src/simplekv
collected 708 items

.../python3.10/site-packages/_pytest/runner.py:136: ResourceWarning: unclosed <socket.socket fd=12, family=AddressFamily.AF_INET, type=SocketKind.SOCK_STREAM, proto=6, laddr=('127.0.0.1', 46992), raddr=('127.0.0.1', 10000)>
    item.funcargs = None  # type: ignore[attr-defined]

... 10000 lines omitted ...

=============================================================================== 572 passed, 136 skipped, 1253 warnings in 34.96s ================================================================================

When using simplekv in another project, it is currently impossible to close these stores manually and one will observe the same warnings. This gets even worse, if the store is wrapped behind multiple decorators so that one has no clue anymore, which type of store is actually instantiated

Solution

In this PR we extend the KeyValueStore and Decorator baseclasses with closing support and implement the baseclass in the _azurestore_new store as an example.

With the implementation we can do

with MyKeyValueStoreClass() as kv:
    kv.dothings()

and it will automatically close. Manual closing via kv.close() is also supported.

With this change (and actually closing the stores) the warning count of test_azure_store.py is now 0 again:

pytest -Wonce tests/test_azure_store.py
============================================================================================== test session starts ==============================================================================================
platform linux -- Python 3.10.6, pytest-7.1.3, pluggy-1.0.0
rootdir: /home/ctfy/src/simplekv
collected 711 items

tests/test_azure_store.py ................................................ssssssssssssssssssssssssssssssssssss......ssssss...............ssssssssssssssssss.............................................. [ 24%]
...................................................................................................................................................................................................ssssss [ 52%]
ssssssssssssssssssssssssssssssssssssssssss........ssssssss.................ssssssssssssssssssss.......................................................................................................... [ 81%]
......................................................................................................................................                                                                    [100%]

======================================================================================= 575 passed, 136 skipped in 35.08s =======================================================================================