fedora-selinux / setroubleshoot

Abandoned, use https://gitlab.com/setroubleshoot
https://gitlab.com/setroubleshoot
9 stars 12 forks source link

dbus api: datetime format #33

Closed dperpeet closed 8 years ago

dperpeet commented 8 years ago

How stable is the API?

In a recent discussion on Cockpit it was noted that the standard for datetime stamps is microseconds since 1970 in a uint64. What do you think?

bachradsusi commented 8 years ago

Cockpit is most likely the only consumer of this API now so I can see no problem with the change. I will prepare a pull request and testing COPR builds in next two days.

bachradsusi commented 8 years ago

setroubleshoot stores first_seen_date and last_seen_date in iso8601 format in its internal database and there's no microseconds in this format.

@dperpeet Would datetime stamps in seconds since Epoch work for you? The change would be trivial then:

--- a/framework/src/setroubleshoot/server.py
+++ b/framework/src/setroubleshoot/server.py
-    @dbus.service.method(dbus_system_interface, sender_keyword="sender", in_signature='s', out_signature='ssiasa(ssssbbi)sss')
+    @dbus.service.method(dbus_system_interface, sender_keyword="sender", in_signature='s', out_signature='ssiasa(ssssbbi)tts')

-                str(alert.first_seen_date), str(alert.last_seen_date), alert.level or ''
+                int(alert.first_seen_date.format("%s")), int(alert.last_seen_date.format("%s")), alert.level or ''
larskarlitski commented 8 years ago

Thanks for considering to change this for cockpit.

There's a strong precedence to use microseconds in D-Bus APIs, because it's a good amount of accuracy that fits into a 64bit integer. For example, systemd, timedated, wpa_supplicant, and many things written with glib/gdbus use that.

You can convert datetime objects into microseconds since the epoch like this:

import time
int(time.mktime(alert.first_seen_date.timetuple())) * 1000000

And please use a unsigned int64 (x) in the type signature.

bachradsusi commented 8 years ago

And please use a unsigned int64 (x) in the type signature.

According to https://dbus.freedesktop.org/doc/dbus-specification.html Unsigned 64-bit integer - UINT64 is represented by 't' type code. Or do I use wrong documentation?

larskarlitski commented 8 years ago

Yes of course, sorry. (I confused it because I looked at timedated last, which uses x to allow for relative time values as well.)

bachradsusi commented 8 years ago

Builds available at https://copr.fedorainfracloud.org/coprs/plautrba/setroubleshoot/build/317080/

bachradsusi commented 8 years ago

I think we should change get_all_alerts_since as well. It accepts strings only in %Y-%m-%dT%H:%M:%SZ format. The new signature will be:

get_all_alerts_since(t: since) -> a(ssi)

where since is number of microseconds since the Epoch

dperpeet commented 8 years ago

I think we should change get_all_alerts_since as well.

Yes, that makes sense!