Python notmuch2 bridge, something changed in the Database constructor

Python notmuch2 bridge, something changed in the Database constructor

[Lele Gaifax]

Hi,

yesterday I found that since some time (sorry, cannot say that more exactly)
my offlineimap's presynchook started to fail, and investigating I have a doubt
about what were wrong.

My script basically looks like this:

  from notmuch2 import Database

  def handle_deleted_messages(db):
      ...

  def move_spam_to_junk(db):
      ...
      
  with Database() as db:
      handle_deleted_messages(db)
      move_spam_to_junk(db)

It surely worked without issue for years, but now I get this:

  >>> from notmuch2 import Database
  >>> db = Database()
  Traceback (most recent call last):
    File "<stdin>", line 1, in <module>
    File "/usr/lib/python3/dist-packages/notmuch2/_database.py", line 160, in __init__
      raise errors.NotmuchError(ret, msg)
  notmuch2.NoDatabaseError: Error: could not locate database.

even if Database doc still says that the path argument can be omitted:

 |  :param path: The directory of where the database is stored.  If
 |     ``None`` the location will be searched according to
 |     :ref:`database`

Sure enough the configuration contains a "database" section, and the
default_path() class method is able to look it up:

  >>> Database.default_path()
  PosixPath('/home/lele/Maildir')

so fixed my problem with a minor tweak, ie doing

  with Database(Database.default_path()) as db:
      handle_deleted_messages(db)
      move_spam_to_junk(db)

I wonder where the trouble is is: has something changed in the notmuch2
bridge, or is the Database documentation out of date?

Thanks in advance and happy xmass everybody!

ciao, lele.
-- 
nickname: Lele Gaifax | Quando vivrò di quello che ho pensato ieri
real: Emanuele Gaifas | comincerò ad aver paura di chi mi copia.
lele@metapensiero.it  |                 -- Fortunato Depero, 1929.
\r

Re: Python notmuch2 bridge, something changed in the Database constructor

[David Bremner]

Lele Gaifax <lele@metapensiero.it> writes:

> It surely worked without issue for years, but now I get this:
>
>   >>> from notmuch2 import Database
>   >>> db = Database()
>   Traceback (most recent call last):
>     File "<stdin>", line 1, in <module>
>     File "/usr/lib/python3/dist-packages/notmuch2/_database.py", line 160, in __init__
>       raise errors.NotmuchError(ret, msg)
>   notmuch2.NoDatabaseError: Error: could not locate database.
>
> even if Database doc still says that the path argument can be omitted:
>
>  |  :param path: The directory of where the database is stored.  If
>  |     ``None`` the location will be searched according to
>  |     :ref:`database`
>
> Sure enough the configuration contains a "database" section, and the
> default_path() class method is able to look it up:
>
>   >>> Database.default_path()
>   PosixPath('/home/lele/Maildir')
>
> so fixed my problem with a minor tweak, ie doing
>
>   with Database(Database.default_path()) as db:
>       handle_deleted_messages(db)
>       move_spam_to_junk(db)
>
> I wonder where the trouble is is: has something changed in the notmuch2
> bridge, or is the Database documentation out of date?

Yes there was a change to the Database constructor in the notmuch2
python module in 0.34.2. It now uses the library to search for
configuration files and databases, so the semantics are a bit
different. But it is also possible that that default for the config
argument should be changed.

Can you try

    with Database(config=Database.CONFIG.SEARCH) as db:

@Floris: when I made the change to the Database constructor, I chose the
default as CONFIG.NONE because this matched the previous semantics
_except_ for finding the database path. But since I don't want to use
the ad hoc default_path method anymore, maybe a better default would be
CONFIG.SEARCH. I guess most users of the python bindings will be OK with
the the configuration values in .notmuch-config being loaded by default.
What do you think?

Re: Python notmuch2 bridge, something changed in the Database constructor

[Lele Gaifax]

David Bremner <david@tethera.net> writes:

> Yes there was a change to the Database constructor in the notmuch2
> python module in 0.34.2. It now uses the library to search for
> configuration files and databases, so the semantics are a bit
> different. But it is also possible that that default for the config
> argument should be changed.
>
> Can you try
>
>     with Database(config=Database.CONFIG.SEARCH) as db:

Yes, that works too. Should the Database docstring be rectified accordingly?

Thank you,
ciao, lele.
-- 
nickname: Lele Gaifax | Quando vivrò di quello che ho pensato ieri
real: Emanuele Gaifas | comincerò ad aver paura di chi mi copia.
lele@metapensiero.it  |                 -- Fortunato Depero, 1929.
\r

Re: Python notmuch2 bridge, something changed in the Database constructor

[Floris Bruynooghe]

On Fri 24 Dec 2021 at 09:03 -0400, David Bremner wrote:
>
> @Floris: when I made the change to the Database constructor, I chose the
> default as CONFIG.NONE because this matched the previous semantics
> _except_ for finding the database path. But since I don't want to use
> the ad hoc default_path method anymore, maybe a better default would be
> CONFIG.SEARCH. I guess most users of the python bindings will be OK with
> the the configuration values in .notmuch-config being loaded by default.
> What do you think?

Originally my intention with opening the database was that doing the
same as notmuch itself does by default would be the default and most
natural thing to do (whether or not I succeeded at the time I have no
idea).  So reading .notmuch-config by default and using it correctly
seems fine to me.  Backwards compatibility is always tricky I guess, but
following notmuch's behaviour itself seems reasonable to me.

As a random data point I just checked my own use and I seem to be
passing the database path explicitly.


Cheers,
Floris

[PATCH] bindings/python-cffi: search for config by default

[David Bremner]

The previous (pre-0.34.2) constructor searched for a config file but
only if the database path was not specified, and only to retrieve
database.path. Neither of the available options (CONFIG.SEARCH or
CONFIG.NONE) matches this semantics exactly, but CONFIG.SEARCH causes
less breakage for people who relied on the old behaviour to set their
database.path [1]. Since it also seems like the friendlier option in
the long run, this commit switches to CONFIG.SEARCH as default.

[1]: id:87fsqijx7u.fsf@metapensiero.it
---
 bindings/python-cffi/notmuch2/_database.py  | 2 +-
 bindings/python-cffi/tests/test_database.py | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/bindings/python-cffi/notmuch2/_database.py b/bindings/python-cffi/notmuch2/_database.py
index 14a8f15c..d7485b4d 100644
--- a/bindings/python-cffi/notmuch2/_database.py
+++ b/bindings/python-cffi/notmuch2/_database.py
@@ -139,7 +139,7 @@ class Database(base.NotmuchObject):
             path = os.fsencode(path)
         return path
 
-    def __init__(self, path=None, mode=MODE.READ_ONLY, config=CONFIG.EMPTY):
+    def __init__(self, path=None, mode=MODE.READ_ONLY, config=CONFIG.SEARCH):
         if isinstance(mode, str):
             mode = self.STR_MODE_MAP[mode]
         self.mode = mode
diff --git a/bindings/python-cffi/tests/test_database.py b/bindings/python-cffi/tests/test_database.py
index 9b3219c0..473723d5 100644
--- a/bindings/python-cffi/tests/test_database.py
+++ b/bindings/python-cffi/tests/test_database.py
@@ -13,7 +13,7 @@ import notmuch2._message as message
 
 @pytest.fixture
 def db(maildir):
-    with dbmod.Database.create(maildir.path) as db:
+    with dbmod.Database.create(maildir.path, config=notmuch2.Database.CONFIG.SEARCH) as db:
         yield db
 
 
-- 
2.34.1

Re: [PATCH] bindings/python-cffi: search for config by default

[Floris Bruynooghe]

On Sat 08 Jan 2022 at 12:19 -0400, David Bremner wrote:

> The previous (pre-0.34.2) constructor searched for a config file but
> only if the database path was not specified, and only to retrieve
> database.path. Neither of the available options (CONFIG.SEARCH or
> CONFIG.NONE) matches this semantics exactly, but CONFIG.SEARCH causes
> less breakage for people who relied on the old behaviour to set their
> database.path [1]. Since it also seems like the friendlier option in
> the long run, this commit switches to CONFIG.SEARCH as default.
>
> [1]: id:87fsqijx7u.fsf@metapensiero.it
> ---
>  bindings/python-cffi/notmuch2/_database.py  | 2 +-
>  bindings/python-cffi/tests/test_database.py | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/bindings/python-cffi/notmuch2/_database.py b/bindings/python-cffi/notmuch2/_database.py
> index 14a8f15c..d7485b4d 100644
> --- a/bindings/python-cffi/notmuch2/_database.py
> +++ b/bindings/python-cffi/notmuch2/_database.py
> @@ -139,7 +139,7 @@ class Database(base.NotmuchObject):
>              path = os.fsencode(path)
>          return path
>  
> -    def __init__(self, path=None, mode=MODE.READ_ONLY, config=CONFIG.EMPTY):
> +    def __init__(self, path=None, mode=MODE.READ_ONLY, config=CONFIG.SEARCH):
>          if isinstance(mode, str):
>              mode = self.STR_MODE_MAP[mode]
>          self.mode = mode
> diff --git a/bindings/python-cffi/tests/test_database.py b/bindings/python-cffi/tests/test_database.py
> index 9b3219c0..473723d5 100644
> --- a/bindings/python-cffi/tests/test_database.py
> +++ b/bindings/python-cffi/tests/test_database.py
> @@ -13,7 +13,7 @@ import notmuch2._message as message
>  
>  @pytest.fixture
>  def db(maildir):
> -    with dbmod.Database.create(maildir.path) as db:
> +    with dbmod.Database.create(maildir.path, config=notmuch2.Database.CONFIG.SEARCH) as db:
>          yield db

LGTM

[PATCH v2] bindings/python-cffi: search for config by default

[David Bremner]

The previous (pre-0.34.2) constructor searched for a config file but
only if the database path was not specified, and only to retrieve
database.path. Neither of the available options (CONFIG.SEARCH or
CONFIG.NONE) matches this semantics exactly, but CONFIG.SEARCH causes
less breakage for people who relied on the old behaviour to set their
database.path [1]. Since it also seems like the friendlier option in
the long run, this commit switches to CONFIG.SEARCH as default.

This requires a certain amount of updating the pytest tests, but most
users will actually have a config file, unlike the test environment.

[1]: id:87fsqijx7u.fsf@metapensiero.it
---
 bindings/python-cffi/notmuch2/_database.py  |  2 +-
 bindings/python-cffi/tests/test_config.py   |  4 ++--
 bindings/python-cffi/tests/test_database.py |  4 ++--
 bindings/python-cffi/tests/test_tags.py     | 15 +++++++++------
 bindings/python-cffi/tests/test_thread.py   |  2 +-
 5 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/bindings/python-cffi/notmuch2/_database.py b/bindings/python-cffi/notmuch2/_database.py
index 14a8f15c..d7485b4d 100644
--- a/bindings/python-cffi/notmuch2/_database.py
+++ b/bindings/python-cffi/notmuch2/_database.py
@@ -139,7 +139,7 @@ class Database(base.NotmuchObject):
             path = os.fsencode(path)
         return path
 
-    def __init__(self, path=None, mode=MODE.READ_ONLY, config=CONFIG.EMPTY):
+    def __init__(self, path=None, mode=MODE.READ_ONLY, config=CONFIG.SEARCH):
         if isinstance(mode, str):
             mode = self.STR_MODE_MAP[mode]
         self.mode = mode
diff --git a/bindings/python-cffi/tests/test_config.py b/bindings/python-cffi/tests/test_config.py
index 1b2695f5..67b0dea4 100644
--- a/bindings/python-cffi/tests/test_config.py
+++ b/bindings/python-cffi/tests/test_config.py
@@ -23,9 +23,9 @@ class TestIter:
 
     def test_set_get(self, maildir):
         # Ensure get-set works from different db objects
-        with dbmod.Database.create(maildir.path) as db0:
+        with dbmod.Database.create(maildir.path, config=dbmod.Database.CONFIG.EMPTY) as db0:
             db0.config['spam'] = 'ham'
-        with dbmod.Database(maildir.path) as db1:
+        with dbmod.Database(maildir.path, config=dbmod.Database.CONFIG.EMPTY) as db1:
             assert db1.config['spam'] == 'ham'
 
     def test_get_keyerror(self, db):
diff --git a/bindings/python-cffi/tests/test_database.py b/bindings/python-cffi/tests/test_database.py
index 9b3219c0..f1d12ea6 100644
--- a/bindings/python-cffi/tests/test_database.py
+++ b/bindings/python-cffi/tests/test_database.py
@@ -13,7 +13,7 @@ import notmuch2._message as message
 
 @pytest.fixture
 def db(maildir):
-    with dbmod.Database.create(maildir.path) as db:
+    with dbmod.Database.create(maildir.path, config=notmuch2.Database.CONFIG.EMPTY) as db:
         yield db
 
 
@@ -293,7 +293,7 @@ class TestQuery:
         maildir.deliver(body='baz',
                         headers=[('In-Reply-To', '<{}>'.format(msgid))])
         notmuch('new')
-        with dbmod.Database(maildir.path, 'rw') as db:
+        with dbmod.Database(maildir.path, 'rw', config=notmuch2.Database.CONFIG.EMPTY) as db:
             yield db
 
     def test_count_messages(self, db):
diff --git a/bindings/python-cffi/tests/test_tags.py b/bindings/python-cffi/tests/test_tags.py
index faf3947b..f2c6209d 100644
--- a/bindings/python-cffi/tests/test_tags.py
+++ b/bindings/python-cffi/tests/test_tags.py
@@ -23,7 +23,7 @@ class TestImmutable:
         """
         maildir.deliver()
         notmuch('new')
-        with database.Database(maildir.path) as db:
+        with database.Database(maildir.path, config=database.Database.CONFIG.EMPTY) as db:
             yield db.tags
 
     def test_type(self, tagset):
@@ -33,7 +33,7 @@ class TestImmutable:
     def test_hash(self, tagset, maildir, notmuch):
         h0 = hash(tagset)
         notmuch('tag', '+foo', '*')
-        with database.Database(maildir.path) as db:
+        with database.Database(maildir.path, config=database.Database.CONFIG.EMPTY) as db:
             h1 = hash(db.tags)
         assert h0 != h1
 
@@ -42,7 +42,7 @@ class TestImmutable:
 
     def test_neq(self, tagset, maildir, notmuch):
         notmuch('tag', '+foo', '*')
-        with database.Database(maildir.path) as db:
+        with database.Database(maildir.path, config=database.Database.CONFIG.EMPTY) as db:
             assert tagset != db.tags
 
     def test_contains(self, tagset):
@@ -159,7 +159,8 @@ class TestMutableTagset:
         _, pathname = maildir.deliver()
         notmuch('new')
         with database.Database(maildir.path,
-                               mode=database.Mode.READ_WRITE) as db:
+                               mode=database.Mode.READ_WRITE,
+                               config=database.Database.CONFIG.EMPTY) as db:
             msg = db.get(pathname)
             yield msg.tags
 
@@ -195,7 +196,8 @@ class TestMutableTagset:
         _, pathname = maildir.deliver(flagged=True)
         notmuch('new')
         with database.Database(maildir.path,
-                               mode=database.Mode.READ_WRITE) as db:
+                               mode=database.Mode.READ_WRITE,
+                               config=database.Database.CONFIG.EMPTY) as db:
             msg = db.get(pathname)
             msg.tags.discard('flagged')
             msg.tags.from_maildir_flags()
@@ -205,7 +207,8 @@ class TestMutableTagset:
         _, pathname = maildir.deliver(flagged=True)
         notmuch('new')
         with database.Database(maildir.path,
-                               mode=database.Mode.READ_WRITE) as db:
+                               mode=database.Mode.READ_WRITE,
+                               config=database.Database.CONFIG.EMPTY) as db:
             msg = db.get(pathname)
             flags = msg.path.name.split(',')[-1]
             assert 'F' in flags
diff --git a/bindings/python-cffi/tests/test_thread.py b/bindings/python-cffi/tests/test_thread.py
index 1f44b35d..afdbcfe0 100644
--- a/bindings/python-cffi/tests/test_thread.py
+++ b/bindings/python-cffi/tests/test_thread.py
@@ -13,7 +13,7 @@ def thread(maildir, notmuch):
     maildir.deliver(body='bar',
                     headers=[('In-Reply-To', '<{}>'.format(msgid))])
     notmuch('new')
-    with notmuch2.Database(maildir.path) as db:
+    with notmuch2.Database(maildir.path, config=notmuch2.Database.CONFIG.EMPTY) as db:
         yield next(db.threads('foo'))
 
 
-- 
2.34.1

Re: [PATCH v2] bindings/python-cffi: search for config by default

[David Bremner]

David Bremner <david@tethera.net> writes:

> The previous (pre-0.34.2) constructor searched for a config file but
> only if the database path was not specified, and only to retrieve
> database.path. Neither of the available options (CONFIG.SEARCH or
> CONFIG.NONE) matches this semantics exactly, but CONFIG.SEARCH causes
> less breakage for people who relied on the old behaviour to set their
> database.path [1]. Since it also seems like the friendlier option in
> the long run, this commit switches to CONFIG.SEARCH as default.

applied the fixed version to master and release. For some reason I only
tested the original version by running pytest, but the notmuch test
suite actually sets up an environment where the code is run without a
config file, and that failed in the initial version.

d