Hi, On Mon, Feb 04, 2013 at 10:42:02PM +0000, Steven Hiscocks wrote: > I've made the suggested changes and pushed it to github. Feedback > welcomed :) Thanks!
Some more thoughts on the API below. Some of those are probably stupid, but I want to throw them out in the open, for your feedback. SD_MESSAGE_* are string constants. Shouldn't they be int constants like in C? The conversion both ways is pretty simple, but if the constants were used outside of journal matches it would be nicer to have them as ints. The downside would be that the user would have to printf the int to use it in a match. But... see next point. It would be nice to expose the rest of sd-id128 API: sd_id128_to_string(3), sd_id128_randomize(3), sd_id128_get_machine(3). They would probably go in a separate module (systemd.id128), since they are useful in writing journal entries too. >>> journal.seek_monotonic(int(monotonic.total_seconds()*1E6), bootid) Python interfaces usually use floating point numbers to mean seconds. A double has ~16 significant numbers, so the accuracy should be enough, so I believe the detail that this is microseconds should be hidden. It would be better to replace PyRun_String with normal C methods, but that can be done later. sd_journal_open_directory is not wrapped, but that can be added later. What about renaming Journalctl to Journal? It doesn't really control anything :) SD_JOURNAL_LOCAL_ONLY should probably be renamed to LOCAL_ONLY (SD_JOURNAL_RUNTIME_ONLY, SYSTEM_ONLY likewise). Here namespaceing will be provided by the module, so there's no need for the long name. Second argument to .seek(), a documentation only change: it would be nice to use io.SEEK_SET, io.SEEK_CUR, io.SEEK_END in the description. Should .query_unique() return a set instead? This would make checking if an field is present faster, and also underline the fact that those are non-repeating entries. Your module will be great for creating a test suite for journal. At the same time it will also serve as a test suite for the module. Zbyszek _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel