Skip to content

Conversation

@xiaochen-zhou
Copy link
Contributor

Purpose of this pull request

Support show cluster members information in seatunnel-cluster scripts

seatunnel-cluster.sh  -cn cluster --member


Member ID                            Address              Role                 Version   
c36ffd44-7cc8-4873-8141-c23efcfce9d4 [localhost]:5801     ACTIVE MASTER        5.1.0     
f2dfaced-25d3-45bd-8974-d1351e45373f [localhost]:5802     MASTER               5.1.0     
322453e5-9f1a-4398-8c7d-5f9db4be1c54 [localhost]:5803     WORKER               5.1.0     
f4e7370e-c7a6-4add-9120-5f9b5e67c159 [localhost]:5804     WORKER               5.1.0     
ae369bdf-2abd-47e5-bd5e-ad54526ee9c8 [localhost]:5805     WORKER               5.1.0 

Does this PR introduce any user-facing change?

Yes

How was this patch tested?

Add e2e tests

Check list

…-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
@github-actions github-actions bot added the core SeaTunnel core module label Jun 26, 2025
@nielifeng nielifeng requested a review from Copilot June 26, 2025 02:48
Copy link
Contributor

Copilot AI left a 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));

Comment on lines +145 to +154
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());
Copy link

Copilot AI Jun 26, 2025

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.

Suggested change
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()));

Copilot uses AI. Check for mistakes.
Copy link
Member

@Hisoka-X Hisoka-X left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add doc

update doc
}

@Test
@DisabledOnJre(value = JRE.JAVA_11, disabledReason = "the test case only works on Java 8")
Copy link
Member

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?

Copy link
Contributor Author

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.

@xiaochen-zhou xiaochen-zhou requested a review from Hisoka-X July 29, 2025 03:41
@corgy-w corgy-w merged commit 6d9e086 into apache:dev Jul 31, 2025
7 checks passed
@xiaochen-zhou xiaochen-zhou deleted the support-show-nodes branch August 31, 2025 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants