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@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@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