SEPURI-SAI-KRISHNA opened a new pull request, #12455: URL: https://github.com/apache/seatunnel/pull/12455
### Purpose of this pull request Two leftovers from the #12416 review, deliberately in one PR rather than two after @davidzollo asked me to consolidate on that thread. **1. `MqttSink` has no Java serialization coverage** (#12416 Issue 2, raised by @DanielLeens). `SeaTunnelSink` extends `Serializable`, so `MqttSink` crosses the same logical DAG boundary as `MqttSource`. That boundary is where a missing field stops a job being submitted at all, which is exactly what a user hit in #12405 and #12406 fixed on the source side. The source now has round trip coverage from #12410, #12416 and #12436. The sink had none. `MqttSinkTest` asserts the catalog table survives by table id and schema, then builds a writer from the restored sink. `pluginConfig` and `seaTunnelRowType` have no getters, so the writer is the only way to observe them: `MqttSinkWriter`'s constructor reads `topic`, `qos`, `batch_size` and `url` from the config and derives its serializer from the row type. `MqttClient` construction is mocked, because that constructor also connects and would otherwise reach `tcp://localhost:1883` from a unit test, the same approach `MqttSinkWriterTest` already uses. A second case pins a value rather than a reference, which is the gap @SEZ9 flagged on the source side in #12436. `qos = 2` is rejected by `MqttSinkWriter` but not by `MqttSink`, so the sink builds and only a writer surfaces it. `MqttSinkOptions.QOS` defaults to `1`, so a `pluginConfig` that came back empty would yield a legal qos and the writer would build without complaint; the rejection only happens if the `2` actually travelled. **2. A comment I got wrong in #12436** (#12436 Issue 1, raised by @DanielLeens). `MqttSourceTest` said `max_queue_size` defaults to 2048. `MqttSourceOptions.MAX_QUEUE_SIZE.defaultValue` is `1000`. 2048 is the engine's intermediate queue size and has nothing to do with this option; I carried it over by mistake. ### On adding a new test class `AGENTS.md` asks not to create test classes casually. There is no existing class that owns `MqttSink`: `MqttSinkFactoryTest` covers the factory's option rule and `MqttSinkWriterTest` covers the writer. This mirrors `MqttSourceTest` on the source side rather than bolting sink-level assertions onto a writer-level class. Happy to fold it into `MqttSinkWriterTest` instead if reviewers prefer, though note #11461 is open against that file. ### Does this PR introduce _any_ user-facing change? No. One new test class and one comment line. `MqttSink`, `MqttSinkWriter`, `MqttSource`, `MqttSourceConfig` and `MqttSourceReader` are byte-identical to `dev`. ### How was this patch tested? ``` ./mvnw -pl seatunnel-connectors-v2/connector-mqtt test Tests run: 36, Failures: 0, Errors: 0, Skipped: 0 ``` `spotless:check` is green. Each of `MqttSink`'s three fields marked `transient` in turn: | Field made transient | Result | |---|---| | `pluginConfig` | both tests fail | | `seaTunnelRowType` | `testSinkIsJavaSerializable` fails | | `catalogTable` | `testSinkIsJavaSerializable` fails | 3 of 3 caught. -- 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]
