[PATCH 1/2] python: add bindings for notmuch_database_get_config{, _list}
Lucas Hoffmann
l-m-h at web.de
Tue Jun 20 13:06:26 PDT 2017
Now I finally found some time to come back to this.
Quoting David Bremner (2017-06-10 13:10:13)
> Thanks for writing these bindings, it will be good to have the bindings
> (almost) catch up to the library again.
My pleasure.
> We generally expect more than just a subject line in the commit message
OK.
> 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).
Done, will send it out the next time I send the patches.
> It would be good to add a couple tests. test/T590-libconfig.sh 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).
Started it, will also come with the next round. I would not remove the
C tests. It could always happen that something is broken with the
python bindings even though the C bindings are fine.
> > + 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?
I don't see a reason to have python programmers handle "manual
iterators" or however you want to call the thing the C code does there.
So I would like to keep *some* simplified interface as well.
It is very easy to turn this into a generator. But then I consider the
name a mismatch. If it is called "get_config_list" it should return a
list. I could add get_config_iterator or get_config_generator and turn
get_config_list into a wrapper:
def get_config_list(self, prefix):
return list(self.get_config_iterator(prefix))
The only problem one could see with this additional entry point is what
you said in id:87wp8kcbvg.fsf at tethera.net about the function
get_all_named_queries (quote below), namely that the names of the python
bindings diverge from the names of the C bindings. I don't think that
there will be a performance problem as I assume that there will not be
many config values in the database so storing them in memory should not
be a problem.
Quoting David Bremner (Message-ID: <87wp8kcbvg.fsf at tethera.net>)
> I have somewhat mixed feelings about this. I don't really like the
> python bindings diverging from the C library. It's also not clear it's
> worth supporting a new API entry (since e.g. if this goes in it also
> needs a test) to save the python client one line of code. On the
> positive side I can see there is arguably a missing abstraction on the
> library side, as those particular config items are special.
I think I will drop this commit
(34d9febc53775a24ca9e1bb1abcef64ea9196b12).
If we want to introduce some more abstract interface in python we could
also do this:
def get_configs(self, prefix):
return dict(self.get_config_iterator(prefix))
(or with a different name like get_config_dict)
Which interface would you prefer?
Lucas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: signature
URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20170620/87ca0e22/attachment.sig>
More information about the notmuch
mailing list