From 246be1bd3c88ba34a8193f864b037299697f7766 Mon Sep 17 00:00:00 2001
From: Paul Natsuo Kishimoto <mail@paul.kishimoto.name>
Date: Tue, 8 Aug 2023 13:10:53 +0200
Subject: [PATCH] Add #488 to docs and release notes

---
 RELEASE_NOTES.rst       |  6 ++++--
 doc/api.rst             |  1 +
 ixmp/core/timeseries.py | 20 ++++++++++++++++----
 ixmp/utils/__init__.py  | 30 +++++++++++++++++++++++++-----
 4 files changed, 46 insertions(+), 11 deletions(-)

diff --git a/RELEASE_NOTES.rst b/RELEASE_NOTES.rst
index 773297461..b7573fa42 100644
--- a/RELEASE_NOTES.rst
+++ b/RELEASE_NOTES.rst
@@ -1,9 +1,11 @@
-.. Next release
-.. ============
+Next release
+============
 
 .. All changes
 .. -----------
 
+- New :func:`.utils.discard_on_error` and matching argument to :meth:`.TimeSeries.transact` to avoid locking :class:`.TimeSeries` / :class:`.Scenario` on failed operations with :class:`.JDBCBackend` (:pull:`488`).
+
 .. _v3.7.0:
 
 v3.7.0 (2023-05-17)
diff --git a/doc/api.rst b/doc/api.rst
index 818a2cb8d..f51f06835 100644
--- a/doc/api.rst
+++ b/doc/api.rst
@@ -205,6 +205,7 @@ Utilities
 
    .. autosummary::
       diff
+      discard_on_error
       format_scenario_list
       maybe_check_out
       maybe_commit
diff --git a/ixmp/core/timeseries.py b/ixmp/core/timeseries.py
index 3bbb2fd25..be2c2462b 100644
--- a/ixmp/core/timeseries.py
+++ b/ixmp/core/timeseries.py
@@ -208,10 +208,22 @@ def transact(
     ):
         """Context manager to wrap code in a 'transaction'.
 
-        If `condition` is :obj:`True`, the TimeSeries (or :class:`.Scenario`) is
-        checked out *before* the block begins. When the block ends, the object is
-        committed with `message`. If `condition` is :obj:`False`, nothing occurs before
-        or after the block.
+        Parameters
+        ----------
+        message : str
+            Commit message to use, if any commit is performed.
+        condition : bool
+            If :obj:`True` (the default):
+
+            - Before entering the code block, the TimeSeries (or :class:`.Scenario`) is
+              checked out.
+            - On exiting the code block normally (without an exception), changes are
+              committed with `message`.
+
+            If :obj:`False`, nothing occurs on entry or exit.
+        discard_on_error : bool
+            If :obj:`True` (default :obj:`False`), then the anti-locking behaviour of
+            :func:`.discard_on_error` also applies to any exception raised in the block.
 
         Example
         -------
diff --git a/ixmp/utils/__init__.py b/ixmp/utils/__init__.py
index e558712b1..7361a723f 100644
--- a/ixmp/utils/__init__.py
+++ b/ixmp/utils/__init__.py
@@ -165,10 +165,26 @@ def diff(a, b, filters=None) -> Iterator[Tuple[str, pd.DataFrame]]:
 
 @contextmanager
 def discard_on_error(ts: "TimeSeries"):
-    """Discard changes to `ts` on any exception and close the database connection.
+    """Context manager to discard changes to `ts` and close the DB on any exception.
 
-    For :mod:`JDBCBackend`, this can avoid leaving a scenario in the "locked" state in
-    the database.
+    For :mod:`JDBCBackend`, this can avoid leaving `ts` in a "locked" state in the
+    database.
+
+    Examples
+    --------
+    >>> mp = ixmp.Platform()
+    >>> s = ixmp.Scenario(mp, ...)
+    >>> with discard_on_error(s):
+    ...     s.add_par(...)  # Any code
+    ...     s.not_a_method()  # Code that raises some exception
+
+    Before the the exception in the final line is raised (and possibly handled by
+    surrounding code):
+
+    - Any changes—for example, here changes due to the call to :meth:`.add_par`—are
+      discarded/not committed;
+    - ``s`` is guaranteed to be in a non-locked state; and
+    - :meth:`.close_db` is called on ``mp``.
     """
     mp = ts.platform
     try:
@@ -178,13 +194,17 @@ def discard_on_error(ts: "TimeSeries"):
             f"Avoid locking {ts!r} before raising {e.__class__.__name__}: "
             + str(e).splitlines()[0].strip('"')
         )
+
         try:
             ts.discard_changes()
+        except Exception:  # pragma: no cover
+            pass  # Some exception trying to discard changes()
+        else:
             log.info(f"Discard {ts.__class__.__name__.lower()} changes")
-        except Exception:
-            pass
+
         mp.close_db()
         log.info("Close database connection")
+
         raise