Close container on deletion #184
Conversation
agoscinski
force-pushed
the
container-del
branch
from
33d5e17 to
cdc05e4
Compare
agoscinski
force-pushed
the
container-del
branch
2 times, most recently
from
f4382c3 to
078caae
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #184 +/- ##
=======================================
Coverage 99.57% 99.57%
=======================================
Files 10 10
Lines 2118 2120 +2
=======================================
+ Hits 2109 2111 +2
Misses 9 9 ☔ View full report in Codecov by Sentry. |
In case the container is not used within a context and not correctly closed. The sessions are now correctly closed if the container is picked up by the garbage collection and deleted.
pylint cannot detect correctly nested generator types, therefore we ignore the errors. Seems related pylint-dev/pylint#9252
agoscinski
force-pushed
the
container-del
branch
from
078caae to
6df0ba2
Compare
tests/test_container.py
Outdated
| @@ -1181,7 +1187,7 @@ def test_get_objects_stream_closes(temp_container, generate_random_data): | |||
| obj_md5s.keys(), skip_if_missing=True | |||
| ) as triplets: | |||
| # I loop over the triplets, but I don't do anything | |||
| for _ in triplets: | |||
| for _ in triplets: # pylint: disable=not-an-iterable | |||
There was a problem hiding this comment.
I think pylint has been upgraded and causes these errors. I verified and the type ares are correct. There seems to be a issue related to this pylint-dev/pylint#9252
I put these into a separate commit
| # We note the number of open of files, since Windows by default has some files open independent of the container | ||
| begin_test_open_files = len(current_process.open_files()) | ||
|
|
||
| temp_container = Container(temp_dir) |
There was a problem hiding this comment.
It is fine for a test, but this means you didn't drop the container after, correct?
There was a problem hiding this comment.
Right should everything should be wrapped by a finally and close the container
| def __del__(self) -> None: | ||
| """Closes all connections on deletion.""" | ||
| self.close() |
There was a problem hiding this comment.
Can you close the resources twice? For example, you already call close() and then you delete the container, will the close called again, what will happened?
There was a problem hiding this comment.
Should work, will add a test
In case the container is not used within a context and not correctly closed. The sessions are still correctly closed if the container is picked up by the garbage collection.