-
Notifications
You must be signed in to change notification settings - Fork 80
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New feature: Add schema metadata to deserialized avro records #174
base: master
Are you sure you want to change the base?
New feature: Add schema metadata to deserialized avro records #174
Conversation
When an Avro record is deserialized to a Clojure map, add the record's name and fully-qualified name to the map as metadata. These record names are potentially useful info that was being discarded. My personal use case for this metadata is to make it easier for consuming code to deal with unions of records. Signed-off-by: Sam Brauer <[email protected]>
src/jackdaw/serdes/avro.clj
Outdated
[field-key (avro->clj field-coercion value)])))) | ||
(vals field->schema+coercion)) | ||
{:name (.getName schema) | ||
:fullname (.getFullName schema)}))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think when hyphenating the FullName
symbol I'd expect to see full-name
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just pushed a commit with the hyphenated name.
Signed-off-by: Sam Brauer <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #174 +/- ##
==========================================
- Coverage 80.92% 80.81% -0.11%
==========================================
Files 39 39
Lines 2364 2367 +3
Branches 149 149
==========================================
Hits 1913 1913
- Misses 302 305 +3
Partials 149 149
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #174 +/- ##
==========================================
+ Coverage 80.92% 80.94% +0.02%
==========================================
Files 39 39
Lines 2364 2367 +3
Branches 149 149
==========================================
+ Hits 1913 1916 +3
Misses 302 302
Partials 149 149
Continue to review full report at Codecov.
|
I hope you agree that there's value in exposing the record names. There may be other ways to do that besides metadata (although it was the best way I could think of), and even with the metadata approach one might have chosen different keywords. |
Hey Sam. Sorry for the radio silence so far. The other person in our company who might have an opinion on this is away on holiday at the moment. I agree that this is valuable information that should be made available and that given the current setup, the metadata seems like the only place to put it. I wonder whether we could just put the whole schema in the metadata (or maybe even both a writer schema and a reader schema). |
Adding the record names only was probably too limiting and opinionated. By exposing the record schema object a consumer has more flexibility to get the names and lots of other data besides. Signed-off-by: Sam Brauer <[email protected]>
No worries. Good to hear that you're open to the general idea. I've also been thinking that exposing the schema itself could be better than exposing the names (since the names and a lot of other info can be obtained from the schema; on the other hand one could argue it's exposing too much internal detail). I must admit that I haven't yet taken the time to digest your recent change related to decoupling the reader and writer schemas. I should do that soon. In the meantime I pushed a commit that adds the schema to the metadata instead of the names. I'll update the PR title as well. |
When an Avro record is deserialized to a Clojure map, add the record's name and fully-qualified name to the map as metadata.These record names are potentially useful info that was being discarded.
When an Avro record is deserialized to a Clojure map, add the record's schema to the map as metadata.
My personal use case for this metadata is to make it easier for consuming code to deal with unions of records.