Re: [systemd-devel] [PATCH] python-systemd: Reader return special fields and _Reader changes
On 14/04/13 20:55, Steven Hiscocks wrote: def get_next(self, skip=1): -Return the next log entry as a dictionary of fields. +Return the next log entry as a mapping type, currently +a standard dictionary of fields. Optional skip value will return the `skip`\-th log entry. Entries will be processed with converters specified during Reader creation. -return self._convert_entry( -super(Reader, self).get_next(skip)) +if super(Reader, self)._next(skip): +entry = super(Reader, self)._get_all() +if entry: +entry['__REALTIME_TIMESTAMP'] = self._get_realtime() +entry['__MONOTONIC_TIMESTAMP'] = self._get_monotonic() +entry['__CURSOR'] = self._get_cursor() I've picked up on a bug here in python3, `_get_cursor` returns a string, which then gets passed to the default bytes-string conversion in `_convert_field` which in turn throws a TypeError. It can be worked around by adding __CUSROR entry to converters, with suitable conversion (direct return or str). I wasn't sure on the correct approach to fix this: - Should `_get_cursor` return bytes, like the non-special fields? - Or should it continue to return string (as it is special, and __{REALTIME,MONOTONIC}_TIMESTAMP fields don't return bytes) and therefore the DEFAULT_CONVERTERS be updated to handle this? - One more complex option is making _convert_field aware of the special fields, and if no entry exists in the converters, simply just returns the value rather than attempting Unicode decode? This would handle the case where some one sets the converters to `{}`. +return self._convert_entry(entry) +return dict() + -- Steven Hiscocks ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] python-systemd: add version number
On 30/04/13 03:33, Zbigniew Jędrzejewski-Szmek wrote: On Tue, Apr 23, 2013 at 08:11:03PM +0100, Steven Hiscocks wrote: From: Steven Hiscocks ste...@hiscocks.me.uk --- Hi, I thought it would be useful to have a version number in the python systemd module. Hi, I haven't replied to this before because of one reservation. Namely, right now all systemd modules are independent, and could be packaged separately, with systemd being an implicit namespace package à la PEP 420. I think that this makes a lot of sense, since systemd itself is composed of many loosely linked parts and and we're unlikely to ever put any functionality in systemd package itself. But adding systemd.__version__ and encouraging people to use it will make such a step harder. OTOH, adding __version__ to individual packages would definitely be worthwhile. As an additional bonus, all those packages have compiled components, so __version__ could be added without any sed postprocessing. Zbyszek Zbyszek, That seems fair to me and I agree it would be worthwhile adding version numbers to the individual packages. :-) -- Steven Hiscocks ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH v2] systemd-python: attach fields to JournalHandler, add SYSLOG_IDENTIFIER
On 24/04/13 03:10, Zbigniew Jędrzejewski-Szmek wrote: Arbitrary fields can be attached at the level of the handler, and they'll be sent with all messages from this handler. This facility is used to attach SYSLOG_IDENTIFIER to all messages, since otherwise journald attaches SYSLOG_IDENTIFIER=python or something similar, which is completely useless. --- This is part (a) in my email. I think it can go in as is, independently of other potential improvements. Zbyszek Like it. I'm particularly keen to see (c) implemented as well, so I went and had a stab at it. I can now see your point Marti about the python logging module not being the easiest to work with without getting ugly very quickly :) I thought I'd share my hacking about for interest. Works by: import logging, sys from systemd import journal logging.setLoggerClass(journal.JournalLogger) log = logging.getLogger(mylogger) log.addHandler(journal.JournalHandler(TEST_FIELD=Default)) log.addHandler(logging.StreamHandler(sys.stdout)) log.warn(Message1) log.warn(Message2, systemd_fields={TEST_FIELD2: Value}) log.warn(Message3, systemd_fields={TEST_FIELD: Not Default}) diff --git a/src/python-systemd/journal.py b/src/python-systemd/journal.py index 7d42525..6a3ad36 100644 --- a/src/python-systemd/journal.py +++ b/src/python-systemd/journal.py @@ -437,6 +437,17 @@ def stream(identifier, priority=LOG_DEBUG, level_prefix=False): fd = stream_fd(identifier, priority, level_prefix) return _os.fdopen(fd, 'w', 1) +class JournalLogger(_logging.Logger): + +def _log(self, *args, **kwargs): +self._systemd_fields = kwargs.pop(systemd_fields, dict()) +super(JournalLogger, self)._log(*args, **kwargs) + +def makeRecord(self, *args, **kwargs): +rv = super(JournalLogger, self).makeRecord(*args, **kwargs) +rv.__dict__[systemd-fields] = self._systemd_fields +return rv + class JournalHandler(_logging.Handler): Journal handler class for the Python logging framework. @@ -508,6 +519,8 @@ class JournalHandler(_logging.Handler): msg = self.format(record) pri = self.mapPriority(record.levelno) mid = getattr(record, 'MESSAGE_ID', None) +extra = self._extra.copy() +extra.update(getattr(record, 'systemd-fields', dict())) send(msg, MESSAGE_ID=mid, PRIORITY=format(pri), @@ -516,7 +529,7 @@ class JournalHandler(_logging.Handler): CODE_FILE=record.pathname, CODE_LINE=record.lineno, CODE_FUNC=record.funcName, - **self._extra) + **extra) except Exception: self.handleError(record) -- Steven Hiscocks ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] python-systemd: add version number
From: Steven Hiscocks ste...@hiscocks.me.uk --- Hi, I thought it would be useful to have a version number in the python systemd module. I'm not overly familiar with Make, etc. but hopefully I've taken the right approach. :) Thanks Steven Hiscocks Makefile.am| 3 +++ src/python-systemd/{__init__.py = __init__.py.in} | 1 + 2 files changed, 4 insertions(+) rename src/python-systemd/{__init__.py = __init__.py.in} (95%) diff --git a/Makefile.am b/Makefile.am index d594a3d..718e6f8 100644 --- a/Makefile.am +++ b/Makefile.am @@ -3868,6 +3868,9 @@ src/%.policy.in: src/%.policy.in.in Makefile %.rules: %.rules.in Makefile $(SED_PROCESS) +src/python-systemd/%: src/python-systemd/%.in + $(SED_PROCESS) + %.sh: %.sh.in Makefile $(SED_PROCESS) $(AM_V_GEN)chmod +x $@ diff --git a/src/python-systemd/__init__.py b/src/python-systemd/__init__.py.in similarity index 95% rename from src/python-systemd/__init__.py rename to src/python-systemd/__init__.py.in index 0d56b99..7109f2a 100644 --- a/src/python-systemd/__init__.py +++ b/src/python-systemd/__init__.py.in @@ -16,3 +16,4 @@ # # You should have received a copy of the GNU Lesser General Public License # along with systemd; If not, see http://www.gnu.org/licenses/. +__version__ = '@PACKAGE_VERSION@' -- 1.8.2.1 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [RFC PATCH] systemd-python: add SYSLOG_IDENTIFIER to JournalHandler
On 22/04/13 21:31, Zbigniew Jędrzejewski-Szmek wrote: Otherwise, we get SYSLOG_IDENTIFIER=python or something similar, which is completely useless. --- Thoughts? Specifically, I'm not sure if is worthwhile to allow overriding SYSLOG_IDENTIFIER per message. Maybe this ability should be removed. Zbyszek src/python-systemd/journal.py | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/python-systemd/journal.py b/src/python-systemd/journal.py index 6c740b0..39db4bb 100644 --- a/src/python-systemd/journal.py +++ b/src/python-systemd/journal.py @@ -469,9 +469,15 @@ class JournalHandler(_logging.Handler): The following journal fields will be sent: `MESSAGE`, `PRIORITY`, `THREAD_NAME`, `CODE_FILE`, `CODE_LINE`, `CODE_FUNC`, `LOGGER` (name as supplied to getLogger call), -`MESSAGE_ID` (optional, see above). +`MESSAGE_ID` (optional, see above), `SYSLOG_IDENTIFIER` (optional). +def __init__(self, level=_logging.NOTSET, SYSLOG_IDENTIFIER=None): +super(JournalHandler, self).__init__(level) +self._SYSLOG_IDENTIFIER = (_sys.argv[0] + if SYSLOG_IDENTIFIER is None + else SYSLOG_IDENTIFIER) + def emit(self, record): Write record as journal event. @@ -485,8 +491,11 @@ class JournalHandler(_logging.Handler): msg = self.format(record) pri = self.mapPriority(record.levelno) mid = getattr(record, 'MESSAGE_ID', None) +sid = getattr(record, 'SYSLOG_IDENTIFIER', + self._SYSLOG_IDENTIFIER) send(msg, MESSAGE_ID=mid, + SYSLOG_IDENTIFIER=sid, PRIORITY=format(pri), LOGGER=record.name, THREAD_NAME=record.threadName, How about having the value as `__name__`, but`_sys.argv[0]` if `__name__ == __main__`? -- Steven Hiscocks ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [RFC PATCH] systemd-python: add SYSLOG_IDENTIFIER to JournalHandler
On 22/04/13 22:15, Zbigniew Jędrzejewski-Szmek wrote: On Mon, Apr 22, 2013 at 09:53:55PM +0100, Steven Hiscocks wrote: +self._SYSLOG_IDENTIFIER = (_sys.argv[0] + if SYSLOG_IDENTIFIER is None + else SYSLOG_IDENTIFIER) How about having the value as `__name__`, but`_sys.argv[0]` if `__name__ == __main__`? __name__ would always be 'journal'. We could use __main__.__name__, but I don't think it makes much of a difference, since it is set from sys.argv[0] anyway. And there's one case where it's less useful: when __name__ == '__main__'. Current code will use SYSLOG_IDENTIFIER='', and journald will add SYSLOG_IDENTIFIER=python on it's own, which looks OK. Zbyszek Oh yeah, lol. I don't think I thought that one through :) -- Steven Hiscocks ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] systemd-python: export new sd_journal_{process, get_events, get_timeout}
On 14/04/13 23:55, Zbigniew Jędrzejewski-Szmek wrote: get_timeout_ms is added as a convenience function, since it is abysmally hard to call clock_gettime() in Python versions lower than 3.3. And even for Python 3.3 users it saves a few lines. --- Hello, I'd apply this directly, but since we're currently discussing changes pin the API, I thought it'd be better if you had a look. Comments, suggestions? Zbyszek Only thought is whether you think it would be good to add poll to the method names for the `get...` ones. e.g. get_poll_events, get_poll_timeout, etc. Otherwise looks good to me :) -- Steven Hiscocks ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Python journal reader
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. What's your handle? Mine's davidstrauss. 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 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...) Hopefully I've covered everything. Feedback welcomed :-) -- Steven Hiscocks ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] python-systemd: Reader return special fields and _Reader changes
From: Steven Hiscocks ste...@hiscocks.me.uk Changes to _Reader make it match closer to C API, by removing `get_next` and `get_previous`. A `get_all` method added, which returns dictionary of fields using C API SD_JOURNAL_FOREACH_DATA macro, which can be used in conjunction with `next`. _Reader `get`, `next`, `get_{realtime,monotonic,cursor}` and new `previous` methods are made private. This is so the traversal and getting of journal fields can be made transparent in the python interface. Reader now solely implements `get_next` and `get_previous`, returning a standard dictionary (future: other mapping types?) with all standard and special fields through the converters. This makes the output the same as journalctl json/export format output. Iterator methods also moved to Reader, as they do not function as intend with changes to _Reader. These changes also mean that more optimised journal interfaces can be made more easily from _Reader, by avoiding getting of unrequired fields by using the `_get` method, and avoiding field conversions. --- src/python-systemd/_reader.c | 90 ++- src/python-systemd/journal.py | 45 -- 2 files changed, 70 insertions(+), 65 deletions(-) diff --git a/src/python-systemd/_reader.c b/src/python-systemd/_reader.c index a49527f..b5f5b9d 100644 --- a/src/python-systemd/_reader.c +++ b/src/python-systemd/_reader.c @@ -260,6 +260,21 @@ static PyObject* Reader_next(Reader *self, PyObject *args) return PyBool_FromLong(r); } +PyDoc_STRVAR(Reader_previous__doc__, + previous([skip]) - bool\n\n + Go to the previous log entry. Optional skip value means to \n + go to the `skip`\\-th previous log entry.\n + Returns False if at start of file, True otherwise.); +static PyObject* Reader_previous(Reader *self, PyObject *args) +{ +int64_t skip = 1LL; +if (!PyArg_ParseTuple(args, |L:previous, skip)) +return NULL; + +return PyObject_CallMethod((PyObject *)self, (char*) _next, + (char*) L, -skip); +} + static int extract(const char* msg, size_t msg_len, PyObject **key, PyObject **value) { @@ -328,11 +343,10 @@ static PyObject* Reader_get(Reader *self, PyObject *args) } -PyDoc_STRVAR(Reader_get_next__doc__, - get_next([skip]) - dict\n\n - Return dictionary of the next log entry. Optional skip value will\n - return the `skip`\\-th log entry. Returns an empty dict on EOF.); -static PyObject* Reader_get_next(Reader *self, PyObject *args) +PyDoc_STRVAR(Reader_get_all__doc__, + _get_all() - dict\n\n + Return dictionary of the current log entry.); +static PyObject* Reader_get_all(Reader *self, PyObject *args) { PyObject _cleanup_Py_DECREF_ *tmp = NULL; PyObject *dict; @@ -340,12 +354,6 @@ static PyObject* Reader_get_next(Reader *self, PyObject *args) size_t msg_len; int r; -tmp = Reader_next(self, args); -if (!tmp) -return NULL; -if (tmp == Py_False) /* EOF */ -return PyDict_New(); - dict = PyDict_New(); if (!dict) return NULL; @@ -465,22 +473,6 @@ static PyObject* Reader_get_monotonic(Reader *self, PyObject *args) return tuple; } - -PyDoc_STRVAR(Reader_get_previous__doc__, - get_previous([skip]) - dict\n\n - Return dictionary of the previous log entry. Optional skip value\n - will return the -`skip`\\-th log entry. Equivalent to get_next(-skip).); -static PyObject* Reader_get_previous(Reader *self, PyObject *args) -{ -int64_t skip = 1LL; -if (!PyArg_ParseTuple(args, |L:get_previous, skip)) -return NULL; - -return PyObject_CallMethod((PyObject *)self, (char*) get_next, - (char*) L, -skip); -} - - PyDoc_STRVAR(Reader_add_match__doc__, add_match(match) - None\n\n Add a match to filter journal log entries. All matches of different\n @@ -710,32 +702,6 @@ static PyObject* Reader_test_cursor(Reader *self, PyObject *args) return PyBool_FromLong(r); } - -static PyObject* Reader_iter(PyObject *self) -{ -Py_INCREF(self); -return self; -} - -static PyObject* Reader_iternext(PyObject *self) -{ -PyObject *dict; -Py_ssize_t dict_size; - -dict = PyObject_CallMethod(self, (char*) get_next, (char*) ); -if (PyErr_Occurred()) -return NULL; -dict_size = PyDict_Size(dict); -if ((int64_t) dict_size 0LL) { -return dict; -} else { -Py_DECREF(dict); -PyErr_SetNone(PyExc_StopIteration); -return NULL; -} -} - - PyDoc_STRVAR(Reader_query_unique__doc__, query_unique(field) - a set of values\n\n Return a set of unique values appearing in journal for the\n @@ -908,12 +874,12 @@ static PyMethodDef Reader_methods[] = { {get_usage, (PyCFunction
Re: [systemd-devel] Python journal reader
On 13/04/13 23:00, David Strauss wrote: If seems like we should put the conditional special handling for __REALTIME_TIMESTAMP and __MONOTONIC_TIMESTAMP in either _reader.c or right in get(). Here's why: * With the code above, calling Reader.get('__REALTIME_TIMESTAMP') results in the wrong type (if anything, I'd need to check), but Reader.get_realtime() gives the right thing. * The type of Reader.get('__REALTIME_TIMESTAMP') is different from Reader.get_next()['__REALTIME_TIMESTAMP'], and it should be equivalent. Good point. Currently, `get` for realtime, monotonic or cursor do not return anything, but raise an exception (invalid field). Given the plan to keep things in `_Reader` close to the C API, I'd suggest adding the handling within the `Reader.get`. Suggestion merged below: ``` diff --git a/src/python-systemd/journal.py b/src/python-systemd/journal.py index 48f57ac..d0ca32b 100644 --- a/src/python-systemd/journal.py +++ b/src/python-systemd/journal.py @@ -189,6 +189,28 @@ class Reader(_Reader): for arg in args: super(Reader, self).add_match(arg) +def get(self, key): +if key == __REALTIME_TIMESTAMP: +return self.get_realtime() +elif key == __MONOTONIC_TIMESTAMP: +return self.get_monotonic() +elif key == __CURSOR: +return self.get_cursor() +else: +return self._convert_field(key, super(Reader, self).get(key)) + +def get_realtime(self): +return self._convert_field( +'__REALTIME_TIMESTAMP', super(Reader, self).get_realtime()) + +def get_monotonic(self): +return self._convert_field( +'__MONOTONIC_TIMESTAMP', super(Reader, self).get_monotonic()) + +def get_cursor(self): +return self._convert_field( +'__CURSOR', super(Reader, self).get_cursor()) + def get_next(self, skip=1): Return the next log entry as a dictionary of fields. @@ -197,8 +219,13 @@ class Reader(_Reader): Entries will be processed with converters specified during Reader creation. -return self._convert_entry( +entry = self._convert_entry( super(Reader, self).get_next(skip)) +entry['__REALTIME_TIMESTAMP'] = self.get_realtime() +entry['__MONOTONIC_TIMESTAMP'] = self.get_monotonic() +entry['__CURSOR'] = self.get_cursor() + +return entry def query_unique(self, field): Return unique values appearing in the journal for given `field`. ``` On Fri, Apr 12, 2013 at 10:13 AM, Steven Hiscocks steven-syst...@hiscocks.me.uk wrote: Hi, In the python journal Reader, the splitting out of monotonic and realtime stamps, has affected `get_next` function as timestamp values are no longer present in the dictionary returned. Also the new `get_monotonic` and `get_realtime` functions are not run through the converters. Equally, the 'get' method added is not run through the converters. I also noted the additional `next` method doesn't work on python2, as it clashes with the iter `next` method (python3 not affected as it changes iter method to `__next__`) My suggestion with the python Reader `get_next` method is that the realtime and monotonic timestamps remain part of it, as these are key parts of a log entry, and two more lines in everyone's code to get them seems cumbersome. Equally also the cursor value. This also makes the output fields the same as the journalctl json format. (I agree it makes sense the _Reader object element to remain separate so close to actual C API). I'm not sure what the best approach to the `next` method issue is… Proposed changes below. I've add `get_cursor` to go through converters for consistency, even if not required currently. ``` ``` Thanks -- Steven Hiscocks ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel -- Steven Hiscocks ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Python journal reader
On 13/04/13 23:47, Steven Hiscocks wrote: On 13/04/13 23:00, David Strauss wrote: If seems like we should put the conditional special handling for __REALTIME_TIMESTAMP and __MONOTONIC_TIMESTAMP in either _reader.c or right in get(). Here's why: * With the code above, calling Reader.get('__REALTIME_TIMESTAMP') results in the wrong type (if anything, I'd need to check), but Reader.get_realtime() gives the right thing. * The type of Reader.get('__REALTIME_TIMESTAMP') is different from Reader.get_next()['__REALTIME_TIMESTAMP'], and it should be equivalent. Good point. Currently, `get` for realtime, monotonic or cursor do not return anything, but raise an exception (invalid field). Given the plan to keep things in `_Reader` close to the C API, I'd suggest adding the handling within the `Reader.get`. Suggestion merged below: ``` ``` On Fri, Apr 12, 2013 at 10:13 AM, Steven Hiscocks steven-syst...@hiscocks.me.uk wrote: Hi, In the python journal Reader, the splitting out of monotonic and realtime stamps, has affected `get_next` function as timestamp values are no longer present in the dictionary returned. Also the new `get_monotonic` and `get_realtime` functions are not run through the converters. Equally, the 'get' method added is not run through the converters. I also noted the additional `next` method doesn't work on python2, as it clashes with the iter `next` method (python3 not affected as it changes iter method to `__next__`) My suggestion with the python Reader `get_next` method is that the realtime and monotonic timestamps remain part of it, as these are key parts of a log entry, and two more lines in everyone's code to get them seems cumbersome. Equally also the cursor value. This also makes the output fields the same as the journalctl json format. (I agree it makes sense the _Reader object element to remain separate so close to actual C API). I'm not sure what the best approach to the `next` method issue is… Proposed changes below. I've add `get_cursor` to go through converters for consistency, even if not required currently. ``` ``` Thanks -- Steven Hiscocks ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel Just noticed an unintended effect if _Reader.get_next returns empty dictionary. Fixed: ``` diff --git a/src/python-systemd/journal.py b/src/python-systemd/journal.py index 48f57ac..281d7c3 100644 --- a/src/python-systemd/journal.py +++ b/src/python-systemd/journal.py @@ -189,6 +189,28 @@ class Reader(_Reader): for arg in args: super(Reader, self).add_match(arg) +def get(self, key): +if key == __REALTIME_TIMESTAMP: +return self.get_realtime() +elif key == __MONOTONIC_TIMESTAMP: +return self.get_monotonic() +elif key == __CURSOR: +return self.get_cursor() +else: +return self._convert_field(key, super(Reader, self).get(key)) + +def get_realtime(self): +return self._convert_field( +'__REALTIME_TIMESTAMP', super(Reader, self).get_realtime()) + +def get_monotonic(self): +return self._convert_field( +'__MONOTONIC_TIMESTAMP', super(Reader, self).get_monotonic()) + +def get_cursor(self): +return self._convert_field( +'__CURSOR', super(Reader, self).get_cursor()) + def get_next(self, skip=1): Return the next log entry as a dictionary of fields. @@ -197,8 +219,14 @@ class Reader(_Reader): Entries will be processed with converters specified during Reader creation. -return self._convert_entry( +entry = self._convert_entry( super(Reader, self).get_next(skip)) +if entry: +entry['__REALTIME_TIMESTAMP'] = self.get_realtime() +entry['__MONOTONIC_TIMESTAMP'] = self.get_monotonic() +entry['__CURSOR'] = self.get_cursor() + +return entry def query_unique(self, field): Return unique values appearing in the journal for given `field`. ``` -- Steven Hiscocks ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] Python journal reader
Hi, In the python journal Reader, the splitting out of monotonic and realtime stamps, has affected `get_next` function as timestamp values are no longer present in the dictionary returned. Also the new `get_monotonic` and `get_realtime` functions are not run through the converters. Equally, the 'get' method added is not run through the converters. I also noted the additional `next` method doesn't work on python2, as it clashes with the iter `next` method (python3 not affected as it changes iter method to `__next__`) My suggestion with the python Reader `get_next` method is that the realtime and monotonic timestamps remain part of it, as these are key parts of a log entry, and two more lines in everyone's code to get them seems cumbersome. Equally also the cursor value. This also makes the output fields the same as the journalctl json format. (I agree it makes sense the _Reader object element to remain separate so close to actual C API). I'm not sure what the best approach to the `next` method issue is… Proposed changes below. I've add `get_cursor` to go through converters for consistency, even if not required currently. ``` diff --git a/src/python-systemd/journal.py b/src/python-systemd/journal.py index 48f57ac..c163ff7 100644 --- a/src/python-systemd/journal.py +++ b/src/python-systemd/journal.py @@ -189,6 +189,21 @@ class Reader(_Reader): for arg in args: super(Reader, self).add_match(arg) +def get(self, key): +return self._convert_field(key, super(Reader, self).get(key)) + +def get_realtime(self): +return self._convert_field( +'__REALTIME_TIMESTAMP', super(Reader, self).get_realtime()) + +def get_monotonic(self): +return self._convert_field( +'__MONOTONIC_TIMESTAMP', super(Reader, self).get_monotonic()) + +def get_cursor(self): +return self._convert_field( +'__CURSOR', super(Reader, self).get_cursor()) + def get_next(self, skip=1): Return the next log entry as a dictionary of fields. @@ -197,8 +212,13 @@ class Reader(_Reader): Entries will be processed with converters specified during Reader creation. -return self._convert_entry( +entry = self._convert_entry( super(Reader, self).get_next(skip)) +entry['__REALTIME_TIMESTAMP'] = self.get_realtime() +entry['__MONOTONIC_TIMESTAMP'] = self.get_monotonic() +entry['__CURSOR'] = self.get_cursor() + +return entry def query_unique(self, field): Return unique values appearing in the journal for given `field`. ``` Thanks -- Steven Hiscocks ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] Journal issues
Hi, I'm having two issues in relation to the journal with my recent upgrade to v201. One issue, is that it appears that _MACHINE_ID field is missing the = character. I had a quick look, and I think the bug was introduced on the last part of the following commit: http://cgit.freedesktop.org/systemd/systemd/commit/src/journal/journald-server.c?id=adb435bb70815461eeddf44dd5d6f1fc2ad9026d The second issue, is it appears that _SOURCE_MONOTONIC_TIMESTAMP is being truncated to just _SOURC. I'm not clear where this issue was introduced. Both issues can be easily seen with journalctl output in export format. I've seen this issue on two of Arch Linux x86_64 systems. Thanks :) -- Steven Hiscocks ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Journal issues
On 13/04/13 00:00, Mirco Tischler wrote: 2013/4/12 Steven Hiscocks steven-syst...@hiscocks.me.uk mailto:steven-syst...@hiscocks.me.uk Hi, I'm having two issues in relation to the journal with my recent upgrade to v201. One issue, is that it appears that _MACHINE_ID field is missing the = character. I had a quick look, and I think the bug was introduced on the last part of the following commit: http://cgit.freedesktop.org/__systemd/systemd/commit/src/__journal/journald-server.c?id=__adb435bb70815461eeddf44dd5d6f1__fc2ad9026d http://cgit.freedesktop.org/systemd/systemd/commit/src/journal/journald-server.c?id=adb435bb70815461eeddf44dd5d6f1fc2ad9026d The second issue, is it appears that _SOURCE_MONOTONIC_TIMESTAMP is being truncated to just _SOURC. I'm not clear where this issue was introduced. Hi My guess is http://cgit.freedesktop.org/systemd/systemd/commit/src/journal/journald-server.c?id=a569398925430de1f8479262e8ab39502054f2e9 Ah. I did look at this and it seemed okay...but now I've seen your patch it's jumped out at me ;) Both issues can be easily seen with journalctl output in export format. I've seen this issue on two of Arch Linux x86_64 systems. Thanks :) A patch to fix this follows up. Thanks Mirco :) On an unrelated note: Is DECIMAL_STR_MAX() in macro.h broken for types longer than 64 bit, or am I just too thick to understand this line: sizeof(type) = 8 ? 20 : sizeof(int[-2*(sizeof(type) 8)]))) ? Thanks Mirco -- Steven Hiscocks ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] python-systemd wait bug
Hi, I noticed an issue in the Reader class within python systemd.journal wait method. Fix is trivial, but patch below to highlight the issue. diff --git a/src/python-systemd/journal.py b/src/python-systemd/journal.py index c918c43..d47a7ba 100644 --- a/src/python-systemd/journal.py +++ b/src/python-systemd/journal.py @@ -221,7 +221,7 @@ class Reader(_Reader): have been added or removed). us = -1 if timeout is None else int(timeout * 100) -return super(Reader, self).wait(timeout) +return super(Reader, self).wait(us) def seek_realtime(self, realtime): Seek to a matching journal entry nearest to `realtime` time. Thanks :) -- Steven Hiscocks ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [systemd-commits] 42 commits - configure.ac Makefile.am man/.gitignore README src/journal src/python-systemd
On 01/03/13 01:23, Zbigniew Jędrzejewski-Szmek wrote: On Thu, Feb 28, 2013 at 05:15:05PM -0800, Zbigniew Jędrzejewski-Szmek wrote: Makefile.am | 80 +++ README |7 configure.ac|1 man/.gitignore |1 src/journal/journalctl.c|9 src/python-systemd/.gitignore |2 src/python-systemd/_journal.c |2 src/python-systemd/_reader.c| 774 src/python-systemd/docs/conf.py | 288 + src/python-systemd/docs/id128.rst | 38 + src/python-systemd/docs/index.rst | 22 + src/python-systemd/docs/journal.rst | 49 ++ src/python-systemd/id128.c | 162 +++ src/python-systemd/journal.py | 299 - src/python-systemd/pyutil.c | 30 + src/python-systemd/pyutil.h | 29 + 16 files changed, 1752 insertions(+), 41 deletions(-) New commits: commit 37d3ab1b7e114f0fb6dfb2e7273569b42794b76a Merge: 54c31a7 7f41820 Author: Zbigniew J??drzejewski-Szmek zbys...@in.waw.pl Date: Thu Feb 28 19:53:42 2013 -0500 Merge branch 'python-systemd-reader' * python-systemd-reader: python-systemd: rename Journal to Reader build-sys: upload python documentation to freedesktop.org systemd-python: add Journal class for reading journal python: build html docs using sphinx journalct: also print Python code in --new-id python: utilize uuid.UUID in logging python: add systemd.id128 module ... and 34 other commits In short: python module systemd.id128 is added, and existing systemd.journal gains a new class systemd.journal.Reader, which can be used to iterate over journal entries. Documentation is provided, and accessible under e.g. pydoc3 systemd.journal.Reader or firefox http://www.freedesktop.org/software/systemd/man/python-systemd/ Hi Steven, as you can see, I merged your module into the master branch. Any additional cleanup can be done in the main tree. Thank you for the module! Zbyszek Brilliant. Thank you for your patience, input and contributions. As you may have noticed I'm a bit of a novice when it comes to C and the python C-API,so you assistance was greatly appreciated. -- Steven Hiscocks ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] python - reading the journal
On 23/02/13 00:43, Zbigniew Jędrzejewski-Szmek wrote: On Thu, Feb 21, 2013 at 06:46:35PM +, Steven Hiscocks wrote: On 21/02/13 01:49, Zbigniew Jędrzejewski-Szmek wrote: On Tue, Feb 19, 2013 at 09:02:56PM +, Steven Hiscocks wrote: I've pushed a few more commits. I've pushed a few commits to https://github.com/keszybz/systemd/commits/python-systemd-reader This is your tree but rebased onto my id128 stuff, and modified to use it. It also has sphinx-generated documentation and gcc all warnings fixed. Please have a look. Zbyszek Thanks Zbyszek. Looks really good. Tidied it up a bit :) With the macros for python 2 and 3 differences: I wonder if unicode_FromString is a bit confusing, as the macro returns a string and not Unicode for python2? Same for long_AsLong, which returns int in python2? I don't think that this matters too much. There's a comment about the naming convention before the macros, and anyway this is purely internal stuff. Also, not related to the commits: I've noted that `sd_journal_get_cursor` man pages states that it returns 0 on success, but it actually returns 1. Note sure if the code or man page is in error. There is a comment in _reader.c get_next against `sd_journal_get_cursor`, which I guess needs to be removed once this is clarified. This has been now fixed in the master branch. Good stuff :) I've pushed a few more commits to https://github.com/keszybz/systemd/commits/python-systemd-reader. Important changes: 1. Checks for return values of all function calls, so now everything should fail nicely, even on OOM conditions. This lengthens the code quite a bit, but seems unavoidable. 2. The value returned for __TIMESTAMP_MONOTONIC: both fields are returned as a tuple. Under Python 3 a named tuple is used: from systemd import journal t = journal.Journal().get_next() t['__MONOTONIC_TIMESTAMP'].timestamp datetime.timedelta(0, 15, 368483) t['__MONOTONIC_TIMESTAMP'].bootid UUID('9838676e-2666-4a83-b8be-b0654747f8be') I think that this is relatively lightweight, yet nice to use. Under Python 2.7, a normal tuple is returned, so it is necessary to use t['__MONOTONIC_TIMESTAMP'][0], t['__MONOTONIC_TIMESTAMP'][1]. I first tried with a class, and it felt too complicated. Like it. Makes sense as the monotonic timestamp is useless without a bootid. I was thinking another way which would allow use of named tuple in both python2.6+ and python3, would be combining the __MONOTONIC_TIMESTAMP and _BOOT_ID together within the `_convert_entry` method within journal.py. I assume that the boot id returned by sd_journal_get_monotonic_usec will always be the same as _BOOT_ID? Or equally you could pass back a plain tuple for __MONOTONIC_TIMESTAMP and then change namedtuple within python _convert_entry? How about changing the `seek_monotonic` method in journal.py to accept a tuple? 3. Sphinx documentation can be generated with 'make destdir-sphinx'. This target is part of 'make doc-sync', which means that the documents will be uploaded along with the man pages to freedesktop.org. So when this is merged http://www.freedesktop.org/software/systemd/man/python-systemd will be the URL with docs. Preview is at: http://keszybz.github.com/systemd/python-systemd/ This way that the docs are generated is not very nice (whole package is installed using random DESTDIR, and sphinx-build is run with PYTHONPATH and LD_LIBRARY_PATH pointing to this directory), but seems to work. The documentation is only generated on request, i.e. only by conscious developers, so it's not catastrophic even if this doesn't always work. I think that this is becoming mergeable. Zbyszek Thanks for all your work on this Zbyszek :) -- Steven Hiscocks ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] python - reading the journal
On 21/02/13 01:49, Zbigniew Jędrzejewski-Szmek wrote: On Tue, Feb 19, 2013 at 09:02:56PM +, Steven Hiscocks wrote: I've pushed a few more commits. I've pushed a few commits to https://github.com/keszybz/systemd/commits/python-systemd-reader This is your tree but rebased onto my id128 stuff, and modified to use it. It also has sphinx-generated documentation and gcc all warnings fixed. Please have a look. Zbyszek Thanks Zbyszek. Looks really good. Tidied it up a bit :) With the macros for python 2 and 3 differences: I wonder if unicode_FromString is a bit confusing, as the macro returns a string and not Unicode for python2? Same for long_AsLong, which returns int in python2? Also, not related to the commits: I've noted that `sd_journal_get_cursor` man pages states that it returns 0 on success, but it actually returns 1. Note sure if the code or man page is in error. There is a comment in _reader.c get_next against `sd_journal_get_cursor`, which I guess needs to be removed once this is clarified. -- Steven Hiscocks ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] python - reading the journal
On 19/02/13 07:05, David Strauss wrote: On Mon, Feb 18, 2013 at 11:28 AM, Steven Hiscocks steven-syst...@hiscocks.me.uk wrote: Sounds good. I suppose `log_level` should become `add_loglevel_matches` for consistency. (or actually `add_priority_matches`?) Log level is the Pythonic language; please continue using it. The journal itself may even move to it because priority is counter-intuitive. I was missing the default value of None in `this_machine`. It's supposed to be used: myjournal.this_machine() (or myjournal.add_machine_match() with above changes). Alternatively, the default None could be removed , and constants set `CURRENT_MACHINE=id128.get_machine()`, etc., which then should be used when calling the method? That sounds even better. Presumably, id128.get_machine() is not too heavy to call once each time the logging code gets imported? I can imagine the overhead is negligible. Looking at the id128 code, it appears to just read /etc/machine-id and /proc/sys/kernel/random/boot_id. (You could even get away with doing this in python ;) ) The ultimate would be CURRENT_MACHINE=something-constant and a function that lazy-loads id128.get_machine() on the first request. It may be a premature optimization for performance, but it's nice to be able to import a Python module without worrying about it throwing an exception initializing its constants. I put in place holder for now for the constants using the id128 functions. Might be something to change if people have issues... I'll add this to the documentation, but I think `get_next` should stay, as it compliments `get_previous` and use of the `skip` value. Makes sense. Sure. I'll put in `UnicodeDecodeError`. I'm undecided on whether the call of `self.converters[key](value)` exception handling should be limited to `KeyError`? This would mean that the whole log entry would fail to return if one of the converters failed. Even a set of explicit exception classes is okay. Something that's a complete surprise will still bubble up. I've left the current code with catch all fall back to Unicode. It doesn't seem right that a whole log entry fails to return, just because one field is skewif. -- I've pushed a few more commits. -- Steven Hiscocks ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] python - reading the journal
On 18/02/13 02:17, David Strauss wrote: Great progress! Thanks, and thank you for your feedback. A few more remarks: * Class methods ought to be verbs. this_machine and this_boot are the key ones that come to mind. Maybe add_machine_match and add_boot_match? Sounds good. I suppose `log_level` should become `add_loglevel_matches` for consistency. (or actually `add_priority_matches`?) * In methods like this_machine that have special handling for the None value as myself, the function should use something more meaningful. You could even define a constant like CURRENT_MACHINE as None. It's just unclear in calling code that myjournal.this_machine(None) matches on the current machine versus, say, no machine field, an empty machine field, or *any* value in the machine field. I was missing the default value of None in `this_machine`. It's supposed to be used: myjournal.this_machine() (or myjournal.add_machine_match() with above changes). Alternatively, the default None could be removed , and constants set `CURRENT_MACHINE=id128.get_machine()`, etc., which then should be used when calling the method? * get_next should note in its documentation that, combined with the native C, one can use a Pythonic iterator directly. I would even make this function private to encourage use of the iterator syntax. I'll add this to the documentation, but I think `get_next` should stay, as it compliments `get_previous` and use of the `skip` value. * The except handler for _convert_unicode() should be more specific. I think it's generally dangerous to have catch-all exceptions, especially ones that fail silently. I couldn't find in the Python docs for 2 or 3 what the exception would be. Sure. I'll put in `UnicodeDecodeError`. I'm undecided on whether the call of `self.converters[key](value)` exception handling should be limited to `KeyError`? This would mean that the whole log entry would fail to return if one of the converters failed. -- David Strauss | da...@davidstrauss.net | +1 512 577 5827 [mobile] On Fri, Feb 15, 2013 at 9:28 AM, Steven Hiscocks steven-syst...@hiscocks.me.uk wrote: On 11/02/13 05:49, Zbigniew Jędrzejewski-Szmek wrote: On Fri, Feb 08, 2013 at 10:20:57PM +, Steven Hiscocks wrote: I thought the easiest way was to just inherit the Journal (now _Journal), replacing __new__ with all the python RunString stuff. https://github.com/kwirk/systemd/commit/9003fdbc69577840ab6a1501a1c49f7377b6124d Hi Steven, comments on your patch: The default conversion must be bytes, otherwise every new binary message field that this module doesn't know about will cause failure. It is better to default to bytes. Currently I've put default to unicode, which falls back to bytes if their is an exception is raised. ... Zbyszek I've been away on a work trip this week so haven't been able to make much progress. I've pushed some changes to a branch on github: https://github.com/kwirk/systemd/tree/python-systemd-reader -- Steven Hiscocks -- David Strauss | da...@davidstrauss.net | +1 512 577 5827 [mobile] -- Steven Hiscocks ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] python - reading the journal
On 11/02/13 05:49, Zbigniew Jędrzejewski-Szmek wrote: On Fri, Feb 08, 2013 at 10:20:57PM +, Steven Hiscocks wrote: I thought the easiest way was to just inherit the Journal (now _Journal), replacing __new__ with all the python RunString stuff. https://github.com/kwirk/systemd/commit/9003fdbc69577840ab6a1501a1c49f7377b6124d Hi Steven, comments on your patch: The default conversion must be bytes, otherwise every new binary message field that this module doesn't know about will cause failure. It is better to default to bytes. Currently I've put default to unicode, which falls back to bytes if their is an exception is raised. ... Zbyszek I've been away on a work trip this week so haven't been able to make much progress. I've pushed some changes to a branch on github: https://github.com/kwirk/systemd/tree/python-systemd-reader -- Steven Hiscocks ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] python - reading the journal
On 06/02/13 00:55, Zbigniew Jędrzejewski-Szmek wrote: On Tue, Feb 05, 2013 at 11:45:10PM +, Steven Hiscocks wrote: On 05/02/13 23:00, Zbigniew Jędrzejewski-Szmek wrote: On Tue, Feb 05, 2013 at 09:22:46PM +, Steven Hiscocks wrote: On 05/02/13 02:49, Zbigniew Jędrzejewski-Szmek wrote: Hi, On Mon, Feb 04, 2013 at 10:42:02PM +, 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 Out of interest, I had a quick go myself :) https://github.com/kwirk/systemd/commit/7207a5547924684bc54eaad0fdff706eec2402a5 -- Steven Hiscocks ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] python - reading the journal
On 08/02/13 20:51, Zbigniew Jędrzejewski-Szmek wrote: On Fri, Feb 08, 2013 at 07:51:48PM +, Steven Hiscocks wrote: On 06/02/13 00:55, Zbigniew Jędrzejewski-Szmek wrote: On Tue, Feb 05, 2013 at 11:45:10PM +, Steven Hiscocks wrote: On 05/02/13 23:00, Zbigniew Jędrzejewski-Szmek wrote: On Tue, Feb 05, 2013 at 09:22:46PM +, Steven Hiscocks wrote: On 05/02/13 02:49, Zbigniew Jędrzejewski-Szmek wrote: Hi, On Mon, Feb 04, 2013 at 10:42:02PM +, 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 Out of interest, I had a quick go myself :) https://github.com/kwirk/systemd/commit/7207a5547924684bc54eaad0fdff706eec2402a5 Looks nice. But I want to clean up those Py_RunString and add full error handling before we merge it. I prepared a patch for the id128 stuff (https://github.com/systemd/systemd/pull/1). David Strauss rightly suggested converting the constants to uuid.UUID. Looking at your module, I'm starting to think that it would be good to split out call_dict/default_func stuff into a separate class in pure Python. It'll have to be either pure Python or quite a bit of C code
Re: [systemd-devel] python - reading the journal
On 05/02/13 02:49, Zbigniew Jędrzejewski-Szmek wrote: Hi, On Mon, Feb 04, 2013 at 10:42:02PM +, 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 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] python - reading the journal
On 05/02/13 23:00, Zbigniew Jędrzejewski-Szmek wrote: On Tue, Feb 05, 2013 at 09:22:46PM +, Steven Hiscocks wrote: On 05/02/13 02:49, Zbigniew Jędrzejewski-Szmek wrote: Hi, On Mon, Feb 04, 2013 at 10:42:02PM +, 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? -- Steven Hiscocks ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] python - reading the journal
On 04/02/13 05:13, David Strauss wrote: I second the interest in committing this to the existing Python support once polished. On Sun, Feb 3, 2013 at 6:07 PM, Zbigniew Jędrzejewski-Szmek zbys...@in.waw.pl wrote: On Sun, Feb 03, 2013 at 09:18:28PM +, Steven Hiscocks wrote: Hi, I've been developing a python module for accessing the journal, using the journal API. (https://github.com/kwirk/pyjournalctl) Great! Have you thought about including it in the systemd repo, once interface nad implementation is stabilized? One issue I've had is with 'sd_journal_seek_monotonic_usec'. When called, followed by call to 'sd_journal_next', I end up at the start of the journal. If I set a match for _BOOT_ID , before (or after) the call to 'sd_journal_seek_monotonic_usec' (with same _BOOT_ID), it then behaves as I would expect and returns the closest log entry to the monotonic time stamp. I was wondering if this is this the intended behaviour, or is this a bug? Looking at the docs, it seems OK. Without _BOOT_ID the behaviour is unspecified. Some comments about the API: journal.add_match(PRIORITY, 5) Why not journal.add_match(PRIORITY=5) ? Likewise journal.add_matches({PRIORITY: 5, _PID: 1}) could be written as journal.add_matches(PRIORITY=5, _PID=1). This would mirror the journal API which is part of the python-systemd module. Note: systemd.journal already has LOG_EMERG and friends (imported from syslog), so there's no need to use plain numbers. Your object-oriented approach, python 2/3 compatiblity, GIL releasing support are great. I would definitely want to pull this into the python-systemd. Zbyszek Thanks both for the positive feedback. So if I understand correctly, a _BOOT_ID match must be in place for sd_journal_seek_monotonic_usec to function correctly? It would be great for it too be included in the systemd repo! I'll look at making the suggested changes. I think that using keyword arguments will make add_matches redundant, so I'll probably drop that and just have the add_match method. -- Steven Hiscocks ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] python - reading the journal
On 04/02/13 18:30, Steven Hiscocks wrote: On 04/02/13 05:13, David Strauss wrote: I second the interest in committing this to the existing Python support once polished. On Sun, Feb 3, 2013 at 6:07 PM, Zbigniew Jędrzejewski-Szmek zbys...@in.waw.pl wrote: On Sun, Feb 03, 2013 at 09:18:28PM +, Steven Hiscocks wrote: Hi, I've been developing a python module for accessing the journal, using the journal API. (https://github.com/kwirk/pyjournalctl) Great! Have you thought about including it in the systemd repo, once interface nad implementation is stabilized? One issue I've had is with 'sd_journal_seek_monotonic_usec'. When called, followed by call to 'sd_journal_next', I end up at the start of the journal. If I set a match for _BOOT_ID , before (or after) the call to 'sd_journal_seek_monotonic_usec' (with same _BOOT_ID), it then behaves as I would expect and returns the closest log entry to the monotonic time stamp. I was wondering if this is this the intended behaviour, or is this a bug? Looking at the docs, it seems OK. Without _BOOT_ID the behaviour is unspecified. Some comments about the API: journal.add_match(PRIORITY, 5) Why not journal.add_match(PRIORITY=5) ? Likewise journal.add_matches({PRIORITY: 5, _PID: 1}) could be written as journal.add_matches(PRIORITY=5, _PID=1). This would mirror the journal API which is part of the python-systemd module. Note: systemd.journal already has LOG_EMERG and friends (imported from syslog), so there's no need to use plain numbers. Your object-oriented approach, python 2/3 compatiblity, GIL releasing support are great. I would definitely want to pull this into the python-systemd. Zbyszek Thanks both for the positive feedback. So if I understand correctly, a _BOOT_ID match must be in place for sd_journal_seek_monotonic_usec to function correctly? It would be great for it too be included in the systemd repo! I'll look at making the suggested changes. I think that using keyword arguments will make add_matches redundant, so I'll probably drop that and just have the add_match method. I've made the suggested changes and pushed it to github. Feedback welcomed :) -- Steven Hiscocks ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] python - reading the journal
Hi, I've been developing a python module for accessing the journal, using the journal API. (https://github.com/kwirk/pyjournalctl) One issue I've had is with 'sd_journal_seek_monotonic_usec'. When called, followed by call to 'sd_journal_next', I end up at the start of the journal. If I set a match for _BOOT_ID , before (or after) the call to 'sd_journal_seek_monotonic_usec' (with same _BOOT_ID), it then behaves as I would expect and returns the closest log entry to the monotonic time stamp. I was wondering if this is this the intended behaviour, or is this a bug? Thanks :) -- Steven Hiscocks ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel