Pick up CAF patch for ISO 8601, add new btest #360
Conversation
Neverlord
force-pushed
the
topic/neverlord/gh-346
branch
3 times, most recently
from
2e03367 to
514e95f
Compare
|
I though |
Neverlord
force-pushed
the
topic/neverlord/gh-346
branch
from
514e95f to
4cc7d15
Compare
Neverlord
force-pushed
the
topic/neverlord/gh-346
branch
from
4cc7d15 to
7143fe8
Compare
|
I'm going to hold off merging this one until after zeek 6.0 is out, just out of interest of keeping the broker release branch in the current state until then. We can backport it later if desired. |
|
Then let's revert this to draft for now. This also changes how Broker formats timestamps that it sends to clients, so we probably should at least have a discussion about how we document/communicate this change. |
|
|
||
| def parse_iso_timestamp(timestamp): | ||
| # replace 'Z' and adjust offset format from '[+-]MM:SS' to '[+-]MMSS' | ||
| # (note: Python versions >= 3.7 don't need this) |
There was a problem hiding this comment.
Following up on our earlier conversation Dominik: you can safely assume 3.7+. We bumped our minimum requirement a while back, see here.
I'm about to submit a round of CI updates since Broker's setup has fallen behind compared to Zeek master and I believe at least the (EOL'd) Ubuntu 18.04 that's currently still configured ran Python 3.6.
There was a problem hiding this comment.
Unfortunately it's not that easy: zeek/zeek#3239
|
6.0 is out, if you want to move this back to a regular PR. |
|
@Neverlord Is this ready for review again? |
|
I thought a bit about this while working on other tasks. Originally this was meant as a bugfix, but it's actually breaking clients. For example @simeonmiteff Go lib. So I think the question is a bit how we go about that breaking change. For the new We also need to discuss the precision of the timestamps. Internally, we have nanosecond precision. However, I think some clients won't be happy if they receive more than 6 digits for the second fraction. At least in Python it seems a bit of a pain. So we could limit the output format to microsecond resolution an also could discuss whether we always print the full six digits or not. |
|
Sorry for the late follow-up here, Dominik. My takes:
Yes, I like that — we should start making use of versioning to handle format changes.
For the Python bindings use-cases I've encountered so far, nanosecond precision seems pretty unimportant. I think 6 digits is sufficient. |
|
Thanks for the feedback! Closing this then. I can probably re-use (at least parts of) the tests, but I'll re-implement the rendering based on the new broker variants later on. |
Closes #346.
@awelzel in a nutshell, these changes are:
Zsuffix[+-]MM:SSsuffixWhen sending a timestamp without UTC offset (or
Zsuffix), Broker will parse the timestamp according to the local timezone.