[systemd-devel] Python journal reader

[David Strauss]

Oh, okay. Sounds good.
On Apr 15, 2013 12:28 AM, "Steven Hiscocks" <steven-systemd at hiscocks.me.uk>
wrote:

> On 15/04/13 04:05, David Strauss wrote:
>
>> Maybe I'm missing something in the Gist patch, but won't
>> Reader.get('__REALTIME_**TIMESTAMP') fail with "field name is not
>> valid"?
>>
>>  You're right, but `get` is changed to `_get`, so is "private". Use
> `get_next` and `get_previous` does on the traversal and getting of both
> special and standard fields to simplify use of the Reader.
>
>> On Sun, Apr 14, 2013 at 10:49 AM, Zbigniew Jędrzejewski-Szmek
>> <zbyszek at in.waw.pl> wrote:
>>
>>> On Sun, Apr 14, 2013 at 03:28:31PM +0100, Steven Hiscocks wrote:
>>>
>>>> On 14/04/13 03:36, David Strauss wrote:
>>>>
>>>>> I keep writing lengthy emails about how we can use this as an
>>>>> opportunity to reduce redundancy and improve consistency, but I should
>>>>> probably ping you on #systemd IRC to hash it out. I can't think of
>>>>> anything elegant that doesn't involve altering the existing journal.py
>>>>> or _reader.c code.
>>>>>
>>>> Hi Steven, hi David,
>>>
>>> I read your discussion on IRC... I agree that backwards compatibility
>>> is not (yet) something that we need to keep very. I think that the
>>> number of people using those Python interfaces is quite small so far,
>>> and having a nice interface is more important than some small breakage.
>>>
>>>  As a starter, I want to enumerate the places where
>>>>> __REALTIME_TIMESTAMP has special handling:
>>>>>   (1) Existing code: A C-based member function in _reader.c.
>>>>>   (2) Existing code: A native Python conversion function in journal.py.
>>>>>   (3) Existing code: An entry in DEFAULT_CONVERTERS.
>>>>>   (4) Added in your patch: A key setting in get_next().
>>>>>   (5) Added in your patch: A Python-based member function in journal.py
>>>>> that overrides (1).
>>>>>   (6) Added in your patch: A condition in get() that invokes (1) when
>>>>> the specific field is requested.
>>>>>
>>>>> While I want to fix this bug, I also don't want six scattered special
>>>>> cases for __REALTIME_TIMESTAMP and similar fields.
>>>>>
>>>>>
>>>> Thanks for the chat earlier on IRC David, especially given your
>>>> localtime :-).
>>>>
>>>> I thought about the points discussed: keeping things similar to
>>>> journalctl -o json output (i.e. include special fields); get,
>>>> get_next, and get_realtime, etc methods create duplication and
>>>> multiple ways to get the same/some values (could cause confusion);
>>>> issue with `next` clashing with python2 __iter__ next; trying to
>>>> stabilise the interface (note: not declared stable yet); performance
>>>> issues with unnecessary dictionary field conversions.
>>>>
>>>> I've therefore created another patch for consideration:
>>>> https://gist.github.com/kwirk/**5382783<https://gist.github.com/kwirk/5382783>
>>>>
>>> This looks pretty nice, and I think it can be committed. Could
>>> you create a patch with a commit message, including some of the
>>> rationale, so that people understand what's happenning? ;)
>>>
>>>  I've removed the get_next and get_previous methods from _Reader, as
>>>> these didn't align with the C API. This is replaced with:`_get_all`
>>>> private method (effectively a wrapper around C API macro
>>>> sd_journal_{enumerate,restart}**_data/SD_JOURNAL_FOREACH_DATA)**;
>>>> renamed `next` method to `_next`, such to avoid name clash (and I
>>>> believe should be private); `_previous` method for consistency.
>>>>
>>>> The `get_next` and `get_previous` methods are now in Reader only.
>>>> These call `_next` method, followed by `_get_all` and then fetch the
>>>> special fields, before passing whole dictionary to converters. This
>>>> makes all the traversal, and getting standard and special fields
>>>> from the journal transparent to the user.
>>>>
>>>> With the changes of removing `get_next` in _Reader, the iter methods
>>>> don't function as you'd expect. To that end, I've moved them to
>>>> Reader with get_next as iter next call as before.
>>>>
>>>> I understand the points about performance, but don't think the
>>>> performance hit is that bad, or critical in most use cases. These
>>>> changes should allow a custom class to inherit _Reader, and the
>>>> create a more optimised version if that is required. (One option if
>>>> this is a major issue, is a custom dictionary class could be
>>>> returned by `get_next`, which uses the __get__ method to lazy
>>>> convert the fields on access. This is starting to get complicated
>>>> thought...)
>>>>
>>> I think we can always do that later on. Maybe it makes sense
>>> to document that get_next() returns something dictionary-like
>>> (collections.abc.Mapping) that right now is just a dict, but
>>> could become something different later on.
>>>
>>> Zbyszek
>>>
>>
>>
>>
>>
>
> --
> Steven Hiscocks
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/systemd-devel/attachments/20130415/bec59fcf/attachment-0001.html>