-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Create Avro extension for Java SDK #24294
Conversation
Codecov Report
@@ Coverage Diff @@
## master #24294 +/- ##
==========================================
- Coverage 73.27% 73.25% -0.02%
==========================================
Files 725 725
Lines 97444 97444
==========================================
- Hits 71403 71385 -18
- Misses 24691 24709 +18
Partials 1350 1350
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
6fc5e88
to
67240a7
Compare
R: @mosche |
Assigning reviewers. If you would like to opt out of this review, comment R: @kennknowles for label java. Available commands:
The PR bot will only process comments in the main thread (not review comments). |
👀 |
Run Spotless PreCommit |
This PR doesn't close the ticket yet, right? |
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.
Some questions, pls have a look @aromanenko-dev
I also found a couple of copy-paste issues. I've checked the diff for all newly committed files. Looks fine except for the cases mentioned below.
find sdks/java -name AvroCoder.java | xargs diff --color
$ grep -r -E 'org\.apache\.beam\.sdk\.(schemas|io)(\.\w+)*\.\w*Avro' sdks/java/extensions/avro
sdks/java/extensions/avro/src/test/java/org/apache/beam/sdk/extensions/avro/schemas/AvroSchemaTest.java: new org.apache.beam.sdk.schemas.AvroRecordSchema()
sdks/java/extensions/avro/src/test/java/org/apache/beam/sdk/extensions/avro/schemas/AvroSchemaTest.java: new org.apache.beam.sdk.schemas.AvroRecordSchema()
sdks/java/extensions/avro/src/test/java/org/apache/beam/sdk/extensions/avro/schemas/AvroSchemaTest.java: new org.apache.beam.sdk.schemas.AvroRecordSchema()
sdks/java/extensions/avro/src/test/java/org/apache/beam/sdk/extensions/avro/schemas/AvroSchemaTest.java: new org.apache.beam.sdk.schemas.AvroRecordSchema()
sdks/java/extensions/avro/src/test/java/org/apache/beam/sdk/extensions/avro/schemas/AvroSchemaTest.java: new org.apache.beam.sdk.schemas.AvroRecordSchema()
sdks/java/extensions/avro/src/test/java/org/apache/beam/sdk/extensions/avro/schemas/AvroSchemaTest.java: new org.apache.beam.sdk.schemas.AvroRecordSchema()
sdks/java/extensions/avro/src/test/java/org/apache/beam/sdk/extensions/avro/schemas/AvroSchemaTest.java: new org.apache.beam.sdk.schemas.AvroRecordSchema()
sdks/java/extensions/avro/src/main/java/org/apache/beam/sdk/extensions/avro/io/AvroIO.java: private static <T> org.apache.beam.sdk.io.AvroSource<T> createSource(
sdks/java/extensions/avro/src/main/java/org/apache/beam/sdk/extensions/avro/io/AvroIO.java: org.apache.beam.sdk.io.AvroSource.@Nullable DatumReaderFactory<T> readerFactory) {
sdks/java/extensions/avro/src/main/java/org/apache/beam/sdk/extensions/avro/io/AvroIO.java: org.apache.beam.sdk.io.AvroSource<?> source =
sdks/java/extensions/avro/src/main/java/org/apache/beam/sdk/extensions/avro/io/AvroIO.java: org.apache.beam.sdk.io.AvroSource.from(filepattern)
sdks/java/extensions/avro/src/main/java/org/apache/beam/sdk/extensions/avro/io/AvroIO.java: ? (org.apache.beam.sdk.io.AvroSource<T>) source.withSchema(schema)
sdks/java/extensions/avro/src/main/java/org/apache/beam/sdk/extensions/avro/io/AvroIO.java: abstract org.apache.beam.sdk.io.AvroSource.@Nullable DatumReaderFactory<T>
sdks/java/extensions/avro/src/main/java/org/apache/beam/sdk/extensions/avro/io/AvroIO.java: org.apache.beam.sdk.io.AvroSource.DatumReaderFactory<T> factory);
sdks/java/extensions/avro/src/main/java/org/apache/beam/sdk/extensions/avro/io/AvroIO.java: org.apache.beam.sdk.io.AvroSource.DatumReaderFactory<T> factory) {
sdks/java/extensions/avro/src/main/java/org/apache/beam/sdk/extensions/avro/io/AvroIO.java: private final org.apache.beam.sdk.io.AvroSource.DatumReaderFactory<T> readerFactory;
sdks/java/extensions/avro/src/main/java/org/apache/beam/sdk/extensions/avro/io/AvroIO.java: org.apache.beam.sdk.io.AvroSource.DatumReaderFactory<T> readerFactory) {
sdks/java/extensions/avro/src/main/java/org/apache/beam/sdk/extensions/avro/io/AvroIO.java: org.apache.beam.sdk.io.AvroSource.from(getFilepattern())
sdks/java/extensions/avro/src/main/java/org/apache/beam/sdk/extensions/avro/io/DynamicAvroDestinations.java: * A specialization of {@link DynamicDestinations} for {@link org.apache.beam.sdk.io.AvroIO}. In
sdks/java/extensions/avro/src/main/java/org/apache/beam/sdk/extensions/avro/io/DynamicAvroDestinations.java: * Return a {@link org.apache.beam.sdk.io.AvroSink.DatumWriterFactory} for a given destination. If
sdks/java/extensions/avro/src/main/java/org/apache/beam/sdk/extensions/avro/io/AvroSink.java
Show resolved
Hide resolved
sdks/java/extensions/avro/src/main/java/org/apache/beam/sdk/extensions/avro/io/AvroIO.java
Outdated
Show resolved
Hide resolved
sdks/java/extensions/avro/src/main/java/org/apache/beam/sdk/extensions/avro/io/AvroIO.java
Outdated
Show resolved
Hide resolved
sdks/java/extensions/avro/src/main/java/org/apache/beam/sdk/extensions/avro/io/AvroIO.java
Show resolved
Hide resolved
...sions/avro/src/main/java/org/apache/beam/sdk/extensions/avro/io/DynamicAvroDestinations.java
Show resolved
Hide resolved
...tensions/avro/src/main/java/org/apache/beam/sdk/extensions/avro/schemas/utils/AvroUtils.java
Outdated
Show resolved
Hide resolved
...xtensions/avro/src/test/java/org/apache/beam/sdk/extensions/avro/schemas/AvroSchemaTest.java
Outdated
Show resolved
Hide resolved
67240a7
to
06dd451
Compare
...sions/avro/src/main/java/org/apache/beam/sdk/extensions/avro/io/DynamicAvroDestinations.java
Outdated
Show resolved
Hide resolved
...sions/avro/src/main/java/org/apache/beam/sdk/extensions/avro/io/DynamicAvroDestinations.java
Outdated
Show resolved
Hide resolved
06dd451
to
8ec7419
Compare
Run Java_GCP_IO_Direct PreCommit |
8ec7419
to
150877e
Compare
Run Java_Pulsar_IO_Direct PreCommit |
@mosche Thanks for review, I believe I addressed all your comments, PTAL |
👀 |
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.
Thanks @aromanenko-dev, looking good now 🎄 ✨
Feel free to merge!
Create a dedicated extension for Java SDK to support Avro format.
This PR mostly includes copy-pasted Avro-related code from
sdks/java/core
with the same package structure and it introduces almost no changes to "core" - only several access modifiers are relaxed to be able to use from other packages. It should not break any other user code.This is the first step to use "Avro from extension" instead of "Avro from core".
Closes #24293
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
R: @username
).addresses #123
), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, commentfixes #<ISSUE NUMBER>
instead.CHANGES.md
with noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI.