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

Close DBrunner db on exit; document the need to close #101

Merged
merged 2 commits into from Dec 7, 2021

Conversation

jtniehof
Copy link
Member

@jtniehof jtniehof commented Dec 7, 2021

The __del__ method of DButils tries to close the database (and log this closure) as a convenience for the user. Unfortunately if the DButils object is still hanging around at program termination, it will be deleted then, and the Python interpreter may have already torn down the logger, and potentially other facilities needed to close the database. The order in which things are torn down is not guaranteed.

This manifests in DBRunner.py since it fails to close the database. I've managed to whack-a-mole away most of the other instances but missed that one.

This PR:

  • Fixes DBRunner to clean up before exit
  • Adds documentation to DButils recommending cleaning up before exit, and also on runMe and ProcessQueue, since they hold references to DButils.

PR Checklist

  • Pull request has descriptive title
  • Pull request gives overview of changes
  • New code has inline comments where necessary
  • Any new modules, functions or classes have docstrings consistent with dbprocessing style
  • (N/A) Major new functionality has appropriate Sphinx documentation
  • (N/A) Added an entry to release notes if fixing a major bug or providing a major new feature
  • (see below) New features and bug fixes should have unit tests
  • (N/A) Relevant issues are linked in the description (use Closes # if this PR closes the issue, or some other reference, such as See # if it is related in some other way)

As this bug is nondeterministic, it can't really be unit tested. Similar treatments have cleared up issues in other cases.

@jtniehof jtniehof added bug documentation Issues that can be addressed with docs (docstrings, sphinx, wiki) labels Dec 7, 2021
@balarsen
Copy link
Contributor

balarsen commented Dec 7, 2021

Good warnings, python is annoying in these.

@dnadeau-lanl dnadeau-lanl merged commit 74cec35 into spacepy:master Dec 7, 2021
@jtniehof jtniehof deleted the log_on_close branch December 7, 2021 21:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug documentation Issues that can be addressed with docs (docstrings, sphinx, wiki)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants