On Thu, Feb 07, 2013 at 03:19:03PM -0800, David Strauss wrote: > Can we spend some time on IRC prepping the systemd patch? I'd like to > get that rolling. Hi David, I pushed an updated version of [1], with uuid.UUID all over. Please have a look.
Zbyszek [1] https://github.com/systemd/systemd/pull/1 > On Wed, Feb 6, 2013 at 3:05 PM, Steven Hiscocks > <steven-syst...@hiscocks.me.uk> 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 > > systemd-devel@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/systemd-devel > > > > -- > David Strauss > | da...@davidstrauss.net > | +1 512 577 5827 [mobile] > _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
