-
Notifications
You must be signed in to change notification settings - Fork 2.2k
[Feature][Core] Support show cluster members information in seatunnel-cluster scripts #9502
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
Conversation
…-cluster scripts [Feature][Core] code style [Feature][Core] add test [Feature][Core] update e2e test [Feature][Core] remove e2e test [Feature][Core] remove e2e test [Feature][Core] remove e2e test
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.
Pull Request Overview
This pull request adds support for displaying cluster members information via the seatunnel-cluster scripts.
- Introduces a new command line option (‑m/‑‑member) to show cluster members.
- Implements the showClusterMembers() logic in ServerExecuteCommand to retrieve and display member details.
- Adds a new e2e test to verify that the correct number of cluster members is returned.
Reviewed Changes
Copilot reviewed 3 out of 5 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| seatunnel-core/seatunnel-starter/src/test/java/org/apache/seatunnel/core/starter/seatunnel/command/ServerExecuteCommandTest.java | Adds e2e test for validating cluster member retrieval. |
| seatunnel-core/seatunnel-starter/src/main/java/org/apache/seatunnel/core/starter/seatunnel/command/ServerExecuteCommand.java | Implements the cluster members display logic and role determination. |
| seatunnel-core/seatunnel-starter/src/main/java/org/apache/seatunnel/core/starter/seatunnel/args/ServerCommandArgs.java | Introduces the new command line argument for showing cluster members. |
Comments suppressed due to low confidence (1)
seatunnel-core/seatunnel-starter/src/main/java/org/apache/seatunnel/core/starter/seatunnel/command/ServerExecuteCommand.java:134
- [nitpick] Consider rephrasing the error message for clarity and consistency (e.g., 'Cluster %s is not running. Please start the cluster first.').
clusterName));
| System.out.printf( | ||
| "%-36s %-20s %-20s %-10s\n", "Member ID", "Address", "Role", "Version"); | ||
|
|
||
| for (Member member : members) { | ||
| System.out.printf( | ||
| "%-36s %-20s %-20s %-10s\n", | ||
| member.getUuid(), | ||
| member.getAddress(), | ||
| getRole(masterMember.getAddress(), member), | ||
| member.getVersion()); |
Copilot
AI
Jun 26, 2025
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.
Consider using the logger (e.g., log.info) instead of System.out.printf for consistent logging practices in production code.
| System.out.printf( | |
| "%-36s %-20s %-20s %-10s\n", "Member ID", "Address", "Role", "Version"); | |
| for (Member member : members) { | |
| System.out.printf( | |
| "%-36s %-20s %-20s %-10s\n", | |
| member.getUuid(), | |
| member.getAddress(), | |
| getRole(masterMember.getAddress(), member), | |
| member.getVersion()); | |
| log.info( | |
| String.format( | |
| "%-36s %-20s %-20s %-10s", | |
| "Member ID", "Address", "Role", "Version")); | |
| for (Member member : members) { | |
| log.info( | |
| String.format( | |
| "%-36s %-20s %-20s %-10s", | |
| member.getUuid(), | |
| member.getAddress(), | |
| getRole(masterMember.getAddress(), member), | |
| member.getVersion())); |
Hisoka-X
left a comment
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.
e46df88 to
e7864b4
Compare
add doc update doc
6af1ae0 to
e0150ff
Compare
| } | ||
|
|
||
| @Test | ||
| @DisabledOnJre(value = JRE.JAVA_11, disabledReason = "the test case only works on Java 8") |
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.
why it only workes on java 8?
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.
why it only workes on java 8?
Sorry, my mistakes. I updated the code.
seatunnel-core/seatunnel-starter/src/main/bin/seatunnel-cluster.sh
Outdated
Show resolved
Hide resolved
seatunnel-core/seatunnel-starter/src/main/bin/seatunnel-cluster.sh
Outdated
Show resolved
Hide resolved
5df3bf4 to
61b8d1e
Compare
Purpose of this pull request
Support show cluster members information in
seatunnel-clusterscriptsDoes this PR introduce any user-facing change?
Yes
How was this patch tested?
Add e2e tests
Check list
New License Guide