Skip to content

Conversation

@steveniemitz
Copy link
Contributor

Similar to PR #11479, it would be useful to be able to explicitly pass a DatumReader factory to AvroIO, and have it use that instead of GenericDatumReader or SpecificDatumReader.

This PR adds withDatumReaderFactory to AvroIO and plumbs it through into AvroSource.

R: @iemejia


Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

  • Choose reviewer(s) and mention them in a comment (R: @username).
  • Format the pull request title like [BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replace BEAM-XXX with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
  • Update CHANGES.md with noteworthy changes.
  • If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

Post-Commit Tests Status (on master branch)

Lang SDK Apex Dataflow Flink Gearpump Samza Spark
Go Build Status --- --- Build Status --- --- Build Status
Java Build Status Build Status Build Status
Build Status
Build Status
Build Status
Build Status
Build Status
Build Status Build Status Build Status
Build Status
Build Status
Python Build Status
Build Status
Build Status
Build Status
--- Build Status
Build Status
Build Status
Build Status
Build Status
--- --- Build Status
XLang --- --- --- Build Status --- --- Build Status

Pre-Commit Tests Status (on master branch)

--- Java Python Go Website
Non-portable Build Status Build Status
Build Status
Build Status Build Status
Portable --- Build Status --- ---

See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.

@steveniemitz steveniemitz force-pushed the avro-source-reader branch from 72ccb34 to c6ae351 Compare May 8, 2020 15:05
@steveniemitz steveniemitz changed the title [BEAM-9931] Allow users of AvroIO to specify a custom DatumReader implementation [BEAM-9931] Support custom avro DaumReaders in AvroIO May 8, 2020
@iemejia iemejia changed the title [BEAM-9931] Support custom avro DaumReaders in AvroIO [BEAM-9931] Support custom Avro DatumReaders in AvroIO May 8, 2020
@iemejia iemejia self-requested a review May 8, 2020 15:17
Copy link
Contributor

@RyanSkraba RyanSkraba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for the feature.

if (codec.equals(DataFileConstants.NULL_CODEC)) {
// Avro can read from a byte[] using a more efficient implementation. If the input is not
// compressed, pass the data in directly.
this.decoder = DecoderFactory.get().binaryDecoder(data, null);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, avoids the input stream wrapper.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat!

Copy link
Member

@iemejia iemejia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Flawless job @steveniemitz Thanks!

if (codec.equals(DataFileConstants.NULL_CODEC)) {
// Avro can read from a byte[] using a more efficient implementation. If the input is not
// compressed, pass the data in directly.
this.decoder = DecoderFactory.get().binaryDecoder(data, null);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat!

@iemejia iemejia merged commit 494256f into apache:master May 13, 2020
@steveniemitz
Copy link
Contributor Author

thanks for the quick reviews!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants