-
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
[YAML] - Kafka write and RAW format #29160
Conversation
This pull request completes the Kafka integration and RAW feature implementation. I conducted a quick test using Dataflow (DF) to validate that everything functions as expected, in addition to the unit tests. The test results indicate that everything is working fine. This is the YAML config I've used (writing here for documenting purposes):
@robertwb Could you please direct me to the location where the YAML documentation is being authored? I've noticed that there are some examples in the README files, but I'm unsure if there is any documentation on the Beam website or if the two are related in any way. |
Codecov Report
@@ Coverage Diff @@
## master #29160 +/- ##
==========================================
- Coverage 38.36% 38.33% -0.04%
==========================================
Files 687 688 +1
Lines 101741 101844 +103
==========================================
+ Hits 39036 39037 +1
- Misses 61126 61228 +102
Partials 1579 1579
Flags with carried forward coverage won't be shown. Click here to find out more. see 5 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Assigning reviewers. If you would like to opt out of this review, comment R: @liferoad for label python. Available commands:
The PR bot will only process comments in the main thread (not review comments). |
Run Python_Coverage PreCommit |
R: @brucearctor |
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control |
@brucearctor the Python tests / Python Unit Tests (macos-lastes, 3.8, py38) has failed (It doesn't make sense to fail) but this one doesn't have the usual "Run" option in the comment section to retry. Do you have permissions to re-run it? |
Looks like all tests are passing now. As for documentation, yeah, I've been putting things in markdown files right in the directory as a start before we're ready to call this stable (which is coming up with 2.52). I've filed #29165 which could probably have sub-issues filed. |
...a/io/kafka/src/main/java/org/apache/beam/sdk/io/kafka/KafkaWriteSchemaTransformProvider.java
Outdated
Show resolved
Hide resolved
...a/io/kafka/src/main/java/org/apache/beam/sdk/io/kafka/KafkaWriteSchemaTransformProvider.java
Outdated
Show resolved
Hide resolved
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!
final SerializableFunction<Row, byte[]> toBytesFn; | ||
if (configuration.getFormat().equals("RAW")) { | ||
int numFields = inputSchema.getFields().size(); | ||
if (numFields != 1) { |
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.
Perhaps check its type as well?
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.
We can do this as a follow-up.
@@ -47,6 +50,9 @@ public class KafkaWriteSchemaTransformProviderTest { | |||
|
|||
private static final Schema BEAMSCHEMA = | |||
Schema.of(Schema.Field.of("name", Schema.FieldType.STRING)); | |||
|
|||
private static final Schema BEAMRAWSCHEMA = |
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.
Nit: Why aren't these CAP_UNDERSCORE_CASE?
addresses #28664
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
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 or the workflows README to see a list of phrases to trigger workflows.