Add header messages (including latched) to EventsFileWriter

[Hackerman342]

No description provided.

[edgarriba]

can you add tests ?

[Hackerman342]

@edgarriba I've spent some time working on adding tests. I went down a rabbithole and ultimately think found that the asyncio test infrastructure is not functioning as we expect. There is an issue when testing the EventServiceRecorder that the record queue cannot be read. Please see #200 for more details

[edgarriba]

LGTM. Question/suggestion: how can we read/parse easily header messages ?

[ethanrublee]

I still feel like this could be simpler.

I can imagine latched topics can generate new data mid recording pretty easily, which would cause this to fail, and a programmer or user to scratch their head.

Also if new meta data is added after a split, header messages will be inconsistent across log files. When is this likely to be needed?

As a user how do I edit the meta data after it's recorded?

I think for latched topics, if we can extend the query string on the sender side, then the latched topics can easily and transparently be carried across log splits with no additional api.

Could meta data just be of the convention /header/foo and be a latched topic?

And then we keep the semantics simple and really don't need to change the APIs?

[Hackerman342]

@ethanrublee

I still feel like this could be simpler.

Yeah I'd like it to be too.

I can imagine latched topics can generate new data mid recording pretty easily, which would cause this to fail, and a programmer or user to scratch their head.

I don't think this does fail .Or maybe I'm misunderstanding what you mean by fail? There's no special casing for latched topics anymore. The only special case is a white list of URIs to consider as header messages, whether it is latched or not. That message comes in, we try to add it as a header, and we write it to the log either way. If we already created a header with that URI in this recording, then the old header is not overwritten. So either way, the new message received is still included in the log.

def subscribe():
        async for event, payload in client.subscribe(subscription, decode=False):
            if uri_to_string(event.uri) in header_uris:
                # Handle header events - typically these will be metadata or calibrations
                self.header_deque.append((event, payload))
            try:
                self.record_queue.put_nowait((event, payload))
            except asyncio.QueueFull:
                self.logger.warning("Queue full, dropping event %s", event)
                continue

Also if new meta data is added after a split, header messages will be inconsistent across log files. When is this likely to be needed?

Likely never. Just thinking through edge cases.

As a user how do I edit the meta data after it's recorded?

We could make a python script with an example for this in farm-ng-amiga when this comes up as a need

I think for latched topics, if we can extend the query string on the sender side, then the latched topics can easily and transparently be carried across log splits with no additional api.

Oh I like this. Instead of a whitelist of subscribed topic URIs, we check for /header as the final component of the Uri path. That simplifies that logic and can remove the change to the proto definition in this PR

Could meta data just be of the convention /header/foo and be a latched topic?

And then we keep the semantics simple and really don't need to change the APIs?

This would require creating a separate service / publisher for the recording app just to publish metadata. I think it makes more sense to keep it as a client and use the request/reply API, but if you'd prefer that direction I can do that,

[Hackerman342]

@edgarriba

LGTM. Question/suggestion: how can we read/parse easily header messages ?

Good idea. I added a method for this in: e790025