gh-111726: Explicitly close database connections in sqlite3 doctests

[sobolevn]

My logic behind this was:

• Never add .close() to .. doctest::, because it distracts people from the core example (except for cases, where it was already present)
• Always add .close() to .. testcode::, because one code part is better than two

• Issue: Doc/library/sqlite3.rst has multiple doctest warnings #111726

---

📚 Documentation preview 📚: https://cpython-previews--111730.org.readthedocs.build/

[sobolevn]

There are still some left:

 Document: library/sqlite3
-------------------------
Exception ignored in: <sqlite3.Connection object at 0x7f4d99de1590>
Traceback (most recent call last):
  File "/home/runner/work/cpython/cpython/Lib/functools.py", line 36, in update_wrapper
    def update_wrapper(wrapper,
ResourceWarning: unclosed database in <sqlite3.Connection object at 0x7f4d99de1590>
Exception ignored in: <sqlite3.Connection object at 0x7f4d99de3ce0>
Traceback (most recent call last):
  File "/home/runner/work/cpython/cpython/Lib/functools.py", line 36, in update_wrapper
    def update_wrapper(wrapper,
ResourceWarning: unclosed database in <sqlite3.Connection object at 0x7f4d99de3ce0>
Exception ignored in: <sqlite3.Connection object at 0x7f4d9dc24160>
Traceback (most recent call last):
  File "/home/runner/work/cpython/cpython/Lib/functools.py", line 36, in update_wrapper
    def update_wrapper(wrapper,
ResourceWarning: unclosed database in <sqlite3.Connection object at 0x7f4d9dc24160>
Exception ignored in: <sqlite3.Connection object at 0x7f4d9dc246b0>
Traceback (most recent call last):
  File "/home/runner/work/cpython/cpython/Lib/functools.py", line 36, in update_wrapper
    def update_wrapper(wrapper,
ResourceWarning: unclosed database in <sqlite3.Connection object at 0x7f4d9dc246b0>
4 items passed all tests:
  72 tests in default
   3 tests in sqlite3.cursor
   3 tests in sqlite3.limits
   8 tests in sqlite3.trace
86 tests in 4 items.
86 passed and 0 failed.
Test passed.
1 items passed all tests:
   9 tests in default (cleanup code)
9 tests in 1 items.
9 passed and 0 failed.
Test passed.
Exception ignored in sys.unraisablehook: <function debug at 0x7f4da10aec90>
Traceback (most recent call last):
  File "<doctest sqlite3.trace[4]>", line 2, in debug
AttributeError: 'sqlite3.Connection' object has no attribute '__name__'
Exception ignored in sys.unraisablehook: <function debug at 0x7f4da10aec90>
Traceback (most recent call last):
  File "<doctest sqlite3.trace[4]>", line 2, in debug
AttributeError: 'sqlite3.Connection' object has no attribute '__name__'

[sobolevn]

One more thing: when running locally I got these files in my Doc/ folder:

» git status
On branch issue-111735
Changes to be committed:
  (use "git restore --staged <file>..." to unstage)
        new file:   Doc/backup.db
        new file:   Doc/dump.sql
        new file:   Doc/example.db
        modified:   Doc/library/turtle.rst
        new file:   Doc/tutorial.db

[erlend-aasland]

One more thing: when running locally I got these files in my Doc/ folder:

» git status

On branch issue-111735

Changes to be committed:

Yeah, we don't clean up temporary files in the doctests. We should, though.

[sobolevn]

@erlend-aasland do you have any idea why do we still have ResourceWarnings? I've looked through the code multiple times, but I cannot find leaking resources.

It is a shame that we cannot debug it :(

[erlend-aasland]

@erlend-aasland do you have any idea why do we still have ResourceWarnings? I've looked through the code multiple time, but I cannot find leaking resources.

I'll have a look tomorrow.

[sobolevn]

Ok, looks like I understood the problem.

Here's the sample code that does this (it is in sqlite3.rst):

      >>> sqlite3.enable_callback_tracebacks(True)
      >>> con = sqlite3.connect(":memory:")
      >>> def evil_trace(stmt):
      ...     5/0
      ...
      >>> con.set_trace_callback(evil_trace)
      >>> def debug(unraisable):
      ...     print(f"{unraisable.exc_value!r} in callback {unraisable.object.__name__}")
      ...     print(f"Error message: {unraisable.err_msg}")
      >>> import sys
      >>> sys.unraisablehook = debug
      >>> cur = con.execute("SELECT 1")
      ZeroDivisionError('division by zero') in callback evil_trace
      Error message: None

[sobolevn]

There are still lots of warnings to fix:

Document: library/sqlite3
-------------------------
4 items passed all tests:
  71 tests in default
   3 tests in sqlite3.cursor
   3 tests in sqlite3.limits
   7 tests in sqlite3.trace
84 tests in 4 items.
84 passed and 0 failed.
Test passed.
2 items passed all tests:
  12 tests in default (cleanup code)
   1 tests in sqlite3.trace (cleanup code)
13 tests in 2 items.
13 passed and 0 failed.
Test passed.
Exception ignored in: <sqlite3.Connection object at 0x127adbf00>
Traceback (most recent call last):
  File "/Users/sobolev/Desktop/cpython2/Doc/venv/lib/python3.13/site-packages/docutils/nodes.py", line 387, in __new__
    def __new__(cls, data, rawsource=None):
ResourceWarning: unclosed database in <sqlite3.Connection object at 0x127adbf00>
Exception ignored in: <sqlite3.Connection object at 0x12740ce20>
Traceback (most recent call last):
  File "/Users/sobolev/Desktop/cpython2/Doc/venv/lib/python3.13/site-packages/docutils/nodes.py", line 387, in __new__
    def __new__(cls, data, rawsource=None):
ResourceWarning: unclosed database in <sqlite3.Connection object at 0x12740ce20>
Exception ignored in: <sqlite3.Connection object at 0x12740ef10>
Traceback (most recent call last):
  File "/Users/sobolev/Desktop/cpython2/Doc/venv/lib/python3.13/site-packages/docutils/nodes.py", line 387, in __new__
    def __new__(cls, data, rawsource=None):
ResourceWarning: unclosed database in <sqlite3.Connection object at 0x12740ef10>
Exception ignored in: <sqlite3.Connection object at 0x1242f9bf0>
Traceback (most recent call last):
  File "/Users/sobolev/Desktop/cpython2/Doc/venv/lib/python3.13/site-packages/docutils/nodes.py", line 387, in __new__
    def __new__(cls, data, rawsource=None):
ResourceWarning: unclosed database in <sqlite3.Connection object at 0x1242f9bf0>
Exception ignored in: <sqlite3.Connection object at 0x123539590>
Traceback (most recent call last):
  File "/Users/sobolev/Desktop/cpython2/Doc/venv/lib/python3.13/site-packages/docutils/nodes.py", line 387, in __new__
    def __new__(cls, data, rawsource=None):
ResourceWarning: unclosed database in <sqlite3.Connection object at 0x123539590>

Document: library/hashlib
-------------------------
1 items passed all tests:
  58 tests in default
58 tests in 1 items.
58 passed and 0 failed.
Test passed.

Document: library/binascii
--------------------------
1 items passed all tests:
   5 tests in default
5 tests in 1 items.
5 passed and 0 failed.
Test passed.
Exception ignored in: <sqlite3.Connection object at 0x1242faad0>
Traceback (most recent call last):
  File "/Users/sobolev/Desktop/cpython2/Doc/venv/lib/python3.13/site-packages/docutils/nodes.py", line 387, in __new__
    def __new__(cls, data, rawsource=None):
ResourceWarning: unclosed database in <sqlite3.Connection object at 0x1242faad0>

[sobolevn]

Now I give up :)
No ideas left.

[vstinner]

@erlend-aasland do you have any idea why do we still have ResourceWarnings?

You can run tests with tracemalloc to see where the object triggering ResourceWarnings was created: https://docs.python.org/dev/library/devmode.html#resourcewarning-example

[vstinner]

Never add .close() to .. doctest::, because it distracts people from the core example (except for cases, where it was already present)

I would prefer to make it explicit that calling close() is recommended. That's why ResourceWarning is emitted if it's not called.

[erlend-aasland]

I would prefer to make it explicit that calling close() is recommended. That's why ResourceWarning is emitted if it's not called.

We should indeed put more emphasis on that in the docs, but there is no reason to do it in every example. There's a lot of examples in the sqlite3 docs, and filling each and everyone of them with best practises is IMO not a good idea for well written docs :)

[vstinner]

LGTM. But I don't get the two :skipif: True. Does it mean that the cleanup is never executed? If yes, just remove it, no?

@erlend-aasland: Are you ok with this PR?

[vstinner]

LGTM

[vstinner]

I'm not sure what's going on with this PR. Once I saw "os.unlink()" calls, then they disappears. Does @sobolevn or @erlend-aasland plan to propose a following PR to call "os.unlink()"? I proposed using support.os_helper.unlink() which is more reliable when used in tests.

[erlend-aasland]

I'm not sure what's going on with this PR. Once I saw "os.unlink()" calls, then they disappears. Does @sobolevn or @erlend-aasland plan to propose a following PR to call "os.unlink()"? I proposed using support.os_helper.unlink() which is more reliable when used in tests.

They were fixed by Hugo in #117604, as I already said in #111730 (comment).

support.os_helper.unlink is not used in any doctests at the moment. Perhaps we should rewrite all relevant doctests to use support.os_helper.unlink?

[erlend-aasland]

@vstinner: Sphinx recommends using its own machinery (doctest_global_cleanup and testcleanup) for cleaning up temporary doctest files:

You can use this to e.g. remove any temporary files that the tests leave behind.

[erlend-aasland]

We cannot land this yet, there are still warnings to be fixed, as Nikita said in #111730 (comment).

[vstinner]

They were fixed by Hugo in #117604,

Great!

as I already said in #111730 (comment).

Oh, I'm lost in GitHub UI which hides comments when a comment is "solved".

support.os_helper.unlink is not used in any doctests at the moment. Perhaps we should rewrite all relevant doctests to use support.os_helper.unlink?

Nah, it's not worth it, os.unlink() is fine. It was just a comment on a review.

We cannot land this yet, there are still warnings to be fixed, as Nikita said in #111730 (comment).

I suggest to merge these fixes to make the situation clearer, since I'm lost in the lost history of this PR. You can, just remove "all" from the PR title :-)

But it's up to you, if you want to first really fix all warnings.

[vstinner]

LGTM

[erlend-aasland]

@sobolevn, @vstinner: I fixed the remaining issues with f170f7c. It seems Sphinx has a problem with doctest and following testcleanup. I chose to fix the issues by adding more explicit .close()s to doctests. It is not bad to promote good behaviour more explicitly.

cc. @AA-Turner

[erlend-aasland]

Sorry it took so long, Nikita!

[bedevere-app]

GH-117630 is a backport of this pull request to the 3.12 branch.

[sobolevn]

@erlend-aasland thanks a lot for your help and for taking this over! 🤝