Can we spend some time on IRC prepping the systemd patch? I'd like to get that rolling.
On Wed, Feb 6, 2013 at 3:05 PM, Steven Hiscocks <[email protected]> wrote: > On 06/02/13 00:55, Zbigniew Jędrzejewski-Szmek wrote: >> >> On Tue, Feb 05, 2013 at 11:45:10PM +0000, Steven Hiscocks wrote: >>> >>> On 05/02/13 23:00, Zbigniew Jędrzejewski-Szmek wrote: >>>> >>>> On Tue, Feb 05, 2013 at 09:22:46PM +0000, Steven Hiscocks wrote: >>>>> >>>>> On 05/02/13 02:49, Zbigniew Jędrzejewski-Szmek wrote: >>>>>> >>>>>> 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. >>>>>> >>>>> Okay. Sounds like they should be dropped from the current code, as >>>>> in the future the SD_MESSAGE_* constants will be accessed via python >>>>> module systemd.id128? >>>> >>>> Yes. >>>> >>>> I think that once pyjournalctl is part of the systemd tree, the >>>> constants >>>> should be generated from sd-messages.h by a script. Otherwise, we'll >>>> be constantly forgetting to update those. >>>> >>>>>>>>> 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. >>>>>> >>>>> Makes sense to me. Done. >>>>>> >>>>>> It would be better to replace PyRun_String with normal C methods, >>>>>> but that can be done later. >>>>>> >>>>> Yeah... I cheated a bit here ;) >>>>>> >>>>>> sd_journal_open_directory is not wrapped, but that can be added >>>>>> later. >>>>>> >>>>> Good point, easy enough to add. Done. >>>>>> >>>>>> What about renaming Journalctl to Journal? It doesn't really control >>>>>> anything :) >>>>>> >>>>> Yeah. I wasn't too sure on the name when I got started. I was >>>>> concious of not clashing with the present systemd.journal. What is >>>>> the overall planned structure for the python modules, and where >>>>> would this fit in? >>>> >>>> Good question. Once the SD_MESSAGE constants are moved, pyjournalctl >>>> will only export Journalctl and a few constants. If think that could >>>> go straight into the systemd.journal module. _journal.so already >>>> links against libsystemd-journal.so.0, so I don't think that the >>>> additional code for Journalctl will make any different. >>>> >>>> Specifically: rename pyjournalctl.c to src/python-systemd/_reader.c >>>> (unless somebody comes up with a better name), and Journalctl to >>>> Journal. >>>> In journal.py import Journal and the constants from _reader. >>>> >>>>>> 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. >>>>>> >>>>> Good point. Done. >>>>> >>>>>> 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. >>>>>> >>>>> I had this in mind when developing, but was just a bit lazy and >>>>> stuck the number in :-p . Done. >>>>>> >>>>>> 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. >>>>>> >>>>> Of course! Done. >>>>>> >>>>>> 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 >>>>>> >>>>> >>>>> Thanks again for the feedback. Latest changes pushed to github. >>>> >>>> Thank you for your work. >>>> >>>> Let me know what you think about the proposed integration scheme. >>>> >>>> Zbyszek >>>> >>> >>> Okay. Sounds good. >>> >>> You'll have to pardon my ignorance :), but my experience of git is >>> limited to use of github... >>> What's the best way to go about achieving this? Should I fork the >>> systemd-repo from freedesktop, putting pyjournalctl.c in as >>> src/python-systemd/_reader.c (and make other changes mentioned) and >>> use `git format-patch` to submit via email? >> >> I'll do it. I need to throughly check if everything compiles anyway. >> >> Zbyszek >> > Sure thing :) > > Just also wanted to give you a heads up that I've also got a pull request on > fail2ban for having it utilise the journal > (https://github.com/fail2ban/fail2ban/pull/82) > > -- > Steven Hiscocks > > _______________________________________________ > systemd-devel mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/systemd-devel -- David Strauss | [email protected] | +1 512 577 5827 [mobile] _______________________________________________ systemd-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/systemd-devel
