Introduces Baseline Header Interface #171
Conversation
It is useful to be able to inspect an Avro header. For example, to assert certain naming conventions are met prior to ingestion. The following changes have been made to expose header values to users: * Header now exposes an interface into its metadata. Said interface users to retrieve the schema's name as a string and retrieve copies of metadata value by key. * Deserializer now stores the parsed header. This new value is used by a new callback, `name`, which reaches into the parsed header and returns the name contained in the header's metadata.
|
@dflemstr Bump. Feedback? |
| @@ -65,20 +66,24 @@ impl<'a, R> Deserializer<'a, R> | |||
| where R: io::Read + read::Limit | |||
| { | |||
| pub fn new(input: R, | |||
| header: &'a header::Header, | |||
There was a problem hiding this comment.
This is a breaking API change, just to highlight that!
There was a problem hiding this comment.
Are you opposed to having a new2 and a new_cow2?
I figure this is safest to ensure we don't break the API.
| borrow::Cow::Borrowed(registry), | ||
| borrow::Cow::Borrowed(schema)) | ||
| } | ||
|
|
||
| fn new_cow(input: R, | ||
| header: borrow::Cow<'a, header::Header>, |
| @@ -116,12 +122,18 @@ impl<'a, R> Deserializer<'a, read::Blocks<R>> | |||
| .ok_or(Error::from(ErrorKind::NoRootType))? | |||
| .into_resolved(®istry); | |||
|
|
|||
| let blocks = read::Blocks::new(input, codec, header.sync.to_vec()); | |||
| let blocks = read::Blocks::new(input, codec, header.clone().sync.to_vec()); | |||
There was a problem hiding this comment.
Do we need to clone the entire header here?
| @@ -1,9 +1,28 @@ | |||
| use serde; | |||
| use std::collections; | |||
| use std::{collections}; | |||
There was a problem hiding this comment.
Yes, this is. I must have added something here and removed it later. I will get rid of the extra {}.
| impl Header { | ||
|
|
||
| /// Returns a copy of the given header's name field as a String. | ||
| pub fn name(&self) -> String { |
There was a problem hiding this comment.
We can return &str here and let the user copy if needed
|
|
||
| /// Returns a copy of the raw bytes stored at the given key | ||
| /// for the given header. | ||
| pub fn get(&self, key: &str) -> Option<serde::bytes::ByteBuf> { |
There was a problem hiding this comment.
Why do we want to copy? Returning reference seems to be more ergonomic. Also the serde buffer type is internal, would prefer &[u8]
|
@dflemstr My turn to get buried in work. :) I plan on addressing your feedback tomorrow. |
|
Going to close this due to inactivity now :) |
It is useful to be able to inspect an Avro header. For
example, to assert certain naming conventions are met prior to ingestion.
The following changes have been made to expose header values to users:
Header now exposes an interface into its metadata. Said interface
users to retrieve the schema's name as a string and retrieve copies of
metadata value by key.
Deserializer now stores the parsed header. This new value is used by
a new callback,
name, which reaches into the parsed header andreturns the name contained in the header's metadata.