[systemd-devel] python - reading the journal

[Steven Hiscocks]

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?
>>>> 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?
> 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.

-- 
Steven Hiscocks