unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* Python notmuch2 bridge, something changed in the Database constructor
@ 2021-12-24  7:09 Lele Gaifax
  2021-12-24 13:03 ` David Bremner
  2022-01-08 16:19 ` [PATCH] bindings/python-cffi: search for config by default David Bremner
  0 siblings, 2 replies; 8+ messages in thread
From: Lele Gaifax @ 2021-12-24  7:09 UTC (permalink / raw)
  To: notmuch

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Python notmuch2 bridge, something changed in the Database constructor
  2021-12-24  7:09 Python notmuch2 bridge, something changed in the Database constructor Lele Gaifax
@ 2021-12-24 13:03 ` David Bremner
  2021-12-24 13:42   ` Lele Gaifax
  2022-01-01 15:20   ` Floris Bruynooghe
  2022-01-08 16:19 ` [PATCH] bindings/python-cffi: search for config by default David Bremner
  1 sibling, 2 replies; 8+ messages in thread
From: David Bremner @ 2021-12-24 13:03 UTC (permalink / raw)
  To: Lele Gaifax, notmuch

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?





^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Python notmuch2 bridge, something changed in the Database constructor
  2021-12-24 13:03 ` David Bremner
@ 2021-12-24 13:42   ` Lele Gaifax
  2022-01-01 15:20   ` Floris Bruynooghe
  1 sibling, 0 replies; 8+ messages in thread
From: Lele Gaifax @ 2021-12-24 13:42 UTC (permalink / raw)
  To: notmuch

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Python notmuch2 bridge, something changed in the Database constructor
  2021-12-24 13:03 ` David Bremner
  2021-12-24 13:42   ` Lele Gaifax
@ 2022-01-01 15:20   ` Floris Bruynooghe
  1 sibling, 0 replies; 8+ messages in thread
From: Floris Bruynooghe @ 2022-01-01 15:20 UTC (permalink / raw)
  To: David Bremner, Lele Gaifax, notmuch

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH] bindings/python-cffi: search for config by default
  2021-12-24  7:09 Python notmuch2 bridge, something changed in the Database constructor Lele Gaifax
  2021-12-24 13:03 ` David Bremner
@ 2022-01-08 16:19 ` David Bremner
  2022-01-08 18:47   ` Floris Bruynooghe
  1 sibling, 1 reply; 8+ messages in thread
From: David Bremner @ 2022-01-08 16:19 UTC (permalink / raw)
  To: Lele Gaifax, notmuch

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] bindings/python-cffi: search for config by default
  2022-01-08 16:19 ` [PATCH] bindings/python-cffi: search for config by default David Bremner
@ 2022-01-08 18:47   ` Floris Bruynooghe
  2022-01-08 21:21     ` [PATCH v2] " David Bremner
  0 siblings, 1 reply; 8+ messages in thread
From: Floris Bruynooghe @ 2022-01-08 18:47 UTC (permalink / raw)
  To: David Bremner, Lele Gaifax, notmuch

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2] bindings/python-cffi: search for config by default
  2022-01-08 18:47   ` Floris Bruynooghe
@ 2022-01-08 21:21     ` David Bremner
  2022-01-09 20:00       ` David Bremner
  0 siblings, 1 reply; 8+ messages in thread
From: David Bremner @ 2022-01-08 21:21 UTC (permalink / raw)
  To: Floris Bruynooghe, David Bremner, Lele Gaifax, notmuch

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] bindings/python-cffi: search for config by default
  2022-01-08 21:21     ` [PATCH v2] " David Bremner
@ 2022-01-09 20:00       ` David Bremner
  0 siblings, 0 replies; 8+ messages in thread
From: David Bremner @ 2022-01-09 20:00 UTC (permalink / raw)
  To: notmuch

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-01-09 20:01 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-24  7:09 Python notmuch2 bridge, something changed in the Database constructor Lele Gaifax
2021-12-24 13:03 ` David Bremner
2021-12-24 13:42   ` Lele Gaifax
2022-01-01 15:20   ` Floris Bruynooghe
2022-01-08 16:19 ` [PATCH] bindings/python-cffi: search for config by default David Bremner
2022-01-08 18:47   ` Floris Bruynooghe
2022-01-08 21:21     ` [PATCH v2] " David Bremner
2022-01-09 20:00       ` David Bremner

Code repositories for project(s) associated with this inbox:

	notmuch.git.git (no URL configured)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).