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

Proposal for closable stores #115

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

Conversation

timostrunk
Copy link

@timostrunk timostrunk commented Sep 19, 2022

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 =======================================================================================

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.

1 participant