SEPURI-SAI-KRISHNA commented on PR #12953: URL: https://github.com/apache/seatunnel/pull/12953#issuecomment-6387817592
@DanielLeens the `Kafka.md` follow-up we agreed on #12927 is now in this PR, as its own commit, so it cannot be lost in a second merge. #12927 merged earlier today as `befdd3655` and added a tombstone section to the Kafka source doc. That section ends with "The `protobuf` format has no such guard and is not covered by this behaviour", which this PR makes untrue. Commit `843121846` updates it, in English and Chinese: - moves `protobuf` into the list of formats that read a tombstone as a record producing no row - removes the "no such guard" sentence, which was the only such claim in either file, leaving none - adds one sentence for the case that stays different: a zero length protobuf value is a valid message in which every field holds its default, so it decodes into a row of defaults rather than being passed over That last point is already pinned by `anEmptyPayloadRemainsAValidMessageOfDefaults` in this PR, which I re-ran on the rebased head: it asserts the row is non null with `0` and `""`. The whole module is 22 tests, 0 failures. I also checked the claim is complete rather than half true. There are two protobuf deserializers, and both produce no row for a tombstone: `ProtobufDeserializationSchema` through the new guard, and `SchemaRegistryAwareProtobufDeserializationSchema` by delegating to it, which is why this PR flips that test from `throwsWhenMessageIsNull` to `returnsNoRowWhenMessageIsNull`. So "the `protobuf` format" in the doc is accurate for both paths. No `incompatible-changes.md` entry: the behaviour moves from failing the job to working, and #12927 merged with no such entry for the identical change on `avro`. The branch is rebased onto `dev` `886eb632f` and is 0 commits behind. **One thing to know before merging.** #12967 edits the same paragraph, to add `compatible_kafka_connect_json`. I confirmed with `git merge-tree` that the two branches conflict on exactly those two files. Whichever merges second needs a one paragraph rebase, and the resolution is to keep both additions. I should also flag that this stretches "one PR, one problem". I did it because you asked for the follow-up explicitly and asked that it not get lost; happy to split the doc commit out if you would rather keep this PR narrow. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
