Re: [PATCH 1/2] python: add bindings for notmuch_database_get_config{, _list}

Subject: Re: [PATCH 1/2] python: add bindings for notmuch_database_get_config{, _list}

Date: Sat, 10 Jun 2017 08:10:13 -0300


Cc: Lucas Hoffmann

From: David Bremner writes:

Thanks for writing these bindings, it will be good to have the bindings
(almost) catch up to the library again.

We generally expect more than just a subject line in the commit message

> +    def get_config(self, key):
> +        """Return the value of the given config key.

I guess we will eventually want set_config as well, even if it's not
needed for your immediate application. It might save future confusion to
add them both at the same time (unless there's something complicated
about adding set_config).

It would be good to add a couple tests. test/ has some
C tests. I think the first one, labelled
"notmuch_database_{set,get}_config" could just be translated into python
(maybe even replace the C test with the python one, depending what
others think).

> +    def get_config_list(self, prefix):

I don't object to the simplified interface, but I would like to know
what we can do if it becomes a performance bottleneck.  Would it be
possible to replace building the list with a generator (yield statement)
without changing client code? or should we take the leap now?