Integrate datum-factory in smb-avro #5181
Conversation
| }); | ||
| ((AvroIO.Sink<ValueT>) AvroIO.sink(getSchema())) | ||
| .withDatumWriterFactory(datumFactory) | ||
| .withCodec(codec.getCodec()); | ||
|
|
||
| if (metadata != null) { | ||
| return sink.withMetadata(metadata); |
There was a problem hiding this comment.
I think there was a bug here: codec was not set if metadata was not null
There was a problem hiding this comment.
ack! good thing there wasn't much adoption of the the metadata feature here... 😬
There was a problem hiding this comment.
BTW, metadata is not exposed on the IO level. Is this expected ?
There was a problem hiding this comment.
it's not part of AvroSortedBucketIO? It should be 😬
There was a problem hiding this comment.
I can add it as a follow-up PR today
| }); | ||
| ((AvroIO.Sink<ValueT>) AvroIO.sink(getSchema())) | ||
| .withDatumWriterFactory(datumFactory) | ||
| .withCodec(codec.getCodec()); | ||
|
|
||
| if (metadata != null) { | ||
| return sink.withMetadata(metadata); |
There was a problem hiding this comment.
ack! good thing there wasn't much adoption of the the metadata feature here... 😬
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5181 +/- ##
=======================================
Coverage 63.05% 63.05%
=======================================
Files 300 300
Lines 10945 10945
Branches 760 760
=======================================
Hits 6901 6901
Misses 4044 4044 ☔ View full report in Codecov by Sentry. |
RustedBones
force-pushed
the
datum-factory-smb
branch
from
b6313aa
to
e060b00
Compare
No description provided.