DanielLeens commented on PR #11754: URL: https://github.com/apache/seatunnel/pull/11754#issuecomment-5340986663
Hi @waterWang, thanks for joining the Slack channel and for the contribution! Just a quick status update on my previous review: the code change itself is correct and low-risk (I confirmed the `TEXT` branch reuses the same `TextDeserializationSchema` pattern the Kafka connector already uses, and the delimiter plumbing is null-safe). The two remaining items from my earlier review are still open, so no new commit has landed yet: 1. Update `docs/en/connectors/source/Pulsar.md` / `docs/zh/connectors/source/Pulsar.md` — the "Multi-table mode only supports JSON, CANAL_JSON and AVRO" line is now stale since `validateFormat` is shared between single- and multi-table paths. 2. Add a small unit test for the new `TEXT` branch (e.g. a `PulsarMultiTableConfigTest`/`PulsarSourceTest` case similar to the existing AVRO ones). Whenever you have time to push those, I'm happy to take another look. One extra heads-up so you don't spend effort unnecessarily: I noticed two other open PRs are tackling this exact same Pulsar `text` format gap — #11792 and #11863. Nothing wrong on your end at all, this kind of overlap just happens naturally when a bug is easy to spot from the docs/option-rule mismatch. For what it's worth, #11792 currently looks like the most complete of the three in review. You may want to keep an eye on which one lands first to avoid duplicate merge conflicts later, but please don't take this as a knock on your PR — the fix here is solid, and finding/fixing real bugs independently is exactly the kind of contribution we want to see more of. Feel free to keep improving this one regardless (the doc + test items above would still be valuable even if another PR merges first), and thanks again for digging into this! -- 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]
