chovy-3012 commented on PR #12698: URL: https://github.com/apache/seatunnel/pull/12698#issuecomment-6099127628
Thanks for the thorough review! All five issues are addressed in the latest commit. **Issue 1 (High):** Factory-level options now use `config.getOptional(X).ifPresent(factory::setX)` instead of `config.get(X) != null`, so setters are only called when the user explicitly sets the option — `jms.*` URI parameters are preserved. Added tests verifying setters are not invoked when options use defaults, and a test with `jms.useAsyncSend=true` in the URI. **Issue 2 (High):** Removed the broker URI from the `write()` exception message; it now contains only the queue name. Updated the test to use a URI with `user:password@` and assert the password does not appear. **Issue 3 (Medium):** Changed `time_to_live` from `intType()` to `longType()`. Added explicit range validation for `delivery_mode` (1 or 2) and `priority` (0–9) in the constructor with clear error messages. Added tests for both invalid values. **Issue 4 (Low):** Added a note to the en/zh docs explaining that `use_async_send = true` or `delivery_mode = 1` can silently lose messages since no `ExceptionListener` is registered. **Issue 5 (Low):** Replaced `catch (Exception ignored)` with `e.addSuppressed(closeError)` so cleanup failures are visible in the stack trace. Added a test verifying the suppressed exception is present. All 50 unit tests pass. Happy to make further adjustments if needed! -- 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]
