[#837] feat: Display information of all application through Cli. #964

slfan1989 · 2023-06-24T04:09:09Z

What changes were proposed in this pull request?

uniffle client-cli -host localhost -port 9526 --applications --app-list application_1,application_2 --appId-regex application_1 -o JSON

sh-3.2# ./uniffle client-cli -host localhost -port 9526 --applications --app-list application_1,application_2 --appId-regex application_1 -o JSON

2023-08-08 18:10:29,130 INFO uniffle.AbstractCustomCommandLine: connected to coordinator: http://localhost:9526.
2023-08-08 18:10:29,517 INFO cli.UniffleCLI: uniffle-client-cli : get applications
application: [{"applicationId":"application_1","user":"test","lastHeartBeatTime":"2023-08-08 18:10:23"}]

uniffle client-cli -host localhost -port 9526 --applications -o JSON

sh-3.2# ./uniffle client-cli -host localhost -port 9526 --applications -o JSON
2023-08-08 18:15:06,737 INFO uniffle.AbstractCustomCommandLine: connected to coordinator: http://localhost:9526.
2023-08-08 18:15:07,149 INFO cli.UniffleCLI: uniffle-client-cli : get applications
application: [{"applicationId":"application_0","user":"test","lastHeartBeatTime":"2023-08-08 18:14:51"},{"applicationId":"application_1","user":"test","lastHeartBeatTime":"2023-08-08 18:14:51"},{"applicationId":"application_10","user":"test","lastHeartBeatTime":"2023-08-08 18:14:51"},{"applicationId":"application_100","user":"test","lastHeartBeatTime":"2023-08-08 18:14:51"},{"applicationId":"application_1000","user":"test","lastHeartBeatTime":"2023-08-08 18:14:51"},{"applicationId":"application_1001","user":"test","lastHeartBeatTime":"2023-08-08 18:14:51"},{"applicationId":"application_1002","user":"test","lastHeartBeatTime":"2023-08-08 18:14:51"},{"applicationId":"application_1003","user":"test","lastHeartBeatTime":"2023-08-08 18:14:51"},{"applicationId":"application_1004","user":"test","lastHeartBeatTime":"2023-08-08 18:14:51"},{"applicationId":"application_1005","user":"test","lastHeartBeatTime":"2023-08-08 18:14:51"}]

sh-3.2# ./uniffle client-cli -host localhost -port 9526 --applications

sh-3.2# ./uniffle client-cli -host localhost -port 9526 --applications
2023-08-08 18:16:32,561 INFO uniffle.AbstractCustomCommandLine: connected to coordinator: http://localhost:9526.
2023-08-08 18:16:33,066 INFO cli.UniffleCLI: uniffle-client-cli : get applications
+-------------------------------------------------------------------+
|                       Uniffle Applications                        |
+------------------+------+---------------------+-------------------+
|  ApplicationId   | User | Last HeartBeatTime  | RemoteStoragePath |
+------------------+------+---------------------+-------------------+
|  application_0   | test | 2023-08-08 18:16:30 |       null        |
|  application_1   | test | 2023-08-08 18:16:30 |       null        |
|  application_10  | test | 2023-08-08 18:16:30 |       null        |
| application_100  | test | 2023-08-08 18:16:30 |       null        |
| application_1000 | test | 2023-08-08 18:16:30 |       null        |
| application_1001 | test | 2023-08-08 18:16:30 |       null        |
| application_1002 | test | 2023-08-08 18:16:30 |       null        |
| application_1003 | test | 2023-08-08 18:16:30 |       null        |
| application_1004 | test | 2023-08-08 18:16:30 |       null        |
| application_1005 | test | 2023-08-08 18:16:30 |       null        |
+------------------+------+---------------------+-------------------+

Why are the changes needed?

fixes #837 [Subtask] Display information of all application through Cli.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Add Junit Test & Test environment verification

… Cli.

slfan1989 · 2023-06-24T04:10:06Z

The code still needs to be improved and will be submitted again soon.

codecov-commenter · 2023-06-24T04:16:57Z

Codecov Report

Attention: Patch coverage is 16.41337% with 275 lines in your changes missing coverage. Please review.

Project coverage is 54.81%. Comparing base (57c35ad) to head (8f90bf0).
Report is 462 commits behind head on master.

Files	Patch %	Lines
...c/main/java/org/apache/uniffle/cli/UniffleCLI.java	26.53%	71 Missing and 1 partial ⚠️
...main/java/org/apache/uniffle/api/AdminRestApi.java	1.85%	53 Missing ⚠️
...in/java/org/apache/uniffle/common/Application.java	0.00%	47 Missing ⚠️
...apache/uniffle/coordinator/ApplicationManager.java	0.00%	43 Missing ⚠️
...iffle/coordinator/web/resource/ServerResource.java	0.00%	19 Missing ⚠️
...java/org/apache/uniffle/client/RestClientImpl.java	0.00%	14 Missing ⚠️
...org/apache/uniffle/entity/ApplicationResponse.java	0.00%	10 Missing ⚠️
.../org/apache/uniffle/AbstractCustomCommandLine.java	47.05%	9 Missing ⚠️
...le/coordinator/web/request/ApplicationRequest.java	68.18%	7 Missing ⚠️
...n/java/org/apache/uniffle/cli/UniffleAdminCLI.java	75.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master     #964      +/-   ##
============================================
+ Coverage     54.27%   54.81%   +0.53%     
- Complexity     2550     2572      +22     
============================================
  Files           386      371      -15     
  Lines         21828    19924    -1904     
  Branches       1807     1863      +56     
============================================
- Hits          11848    10921     -927     
+ Misses         9278     8369     -909     
+ Partials        702      634      -68

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

… Cli.

slfan1989 · 2023-06-25T23:45:49Z

@jerqi The function of the code has been completed, and I will add unit tests and screenshots of the test environment as soon as possible.

FormattingCLIUtils.java(apache/hadoop#5577) can help the command line to format the output, so that the content of the command line output has better readability. FormattingCLIUtils.java was contributed by @yl09099 , I think this class should be submitted by him.

advancedxy · 2023-06-26T02:56:50Z

is it possible to output applications in json format? That way, it would be easier to pipe the result to other command line tools.

slfan1989 · 2023-06-26T04:55:54Z

is it possible to output applications in json format? That way, it would be easier to pipe the result to other command line tools.

Thanks for the suggestion! I will try to use json to output the result.

advancedxy · 2023-06-26T05:10:05Z

is it possible to output applications in json format? That way, it would be easier to pipe the result to other command line tools.

Thanks for the suggestion! I will try to use json to output the result.

I think current output is fine. It’s nice to have an option to output in json format.

slfan1989 · 2023-06-27T15:17:45Z

I think current output is fine. It’s nice to have an option to output in json format.

Thanks for your suggestions! I will improve this part of the code.

smallzhongfeng

Thanks @slfan1989 Gernally lgtm except for some minor comment ! Could you resolve these conflicts first?

smallzhongfeng · 2023-07-09T09:42:43Z

cli/src/main/java/org/apache/uniffle/cli/UniffleCLI.java

+
+    if (cmd.hasOption(uniffleApplicationCli.getOpt())) {
+      LOG.info("uniffle-client-cli : get applications");
+      PrintWriter writer = new PrintWriter(new OutputStreamWriter(


Could we use try-resource? I think we need to close these stream finally.

I will modify code.

smallzhongfeng · 2023-07-09T09:43:10Z

cli/src/main/java/org/apache/uniffle/cli/UniffleCLI.java

+    uniffleApplicationCli = new Option(shortPrefix + "apps", longPrefix + "applications",
+        true, "The command will be used to print a list of applications. \n"
+         + "We usually use the command like this: \n"
+         + "uniffle -apps|--applications application_167671938823_0001,application_167671938823_0002");


smallzhongfeng · 2023-07-09T09:45:15Z

cli/src/main/java/org/apache/uniffle/client/RestClientImpl.java

@@ -45,6 +50,7 @@ public class RestClientImpl implements RestClient {
  private static final Logger LOG = LoggerFactory.getLogger(RestClientImpl.class);
  private CloseableHttpClient httpclient;
  private String baseUrl;
+  final ObjectMapper mapper = new ObjectMapper().setSerializationInclusion(JsonInclude.Include.NON_NULL);


Should this need to be private?

Thanks for reviewing the code, I will modify code.

jerqi

LGTM, thanks @smallzhongfeng @advancedxy @slfan1989

advancedxy · 2023-07-18T11:16:05Z

Seems like there are still some merge conflict, would you mind to resolve it first?

slfan1989 · 2023-07-19T05:08:13Z

@advancedxy @smallzhongfeng @jerqi Can you help to review this PR again? Thank you very much!

jerqi · 2023-08-05T17:08:35Z

cli/pom.xml

@@ -49,5 +49,9 @@
            <groupId>org.apache.httpcomponents</groupId>
            <artifactId>httpclient</artifactId>
        </dependency>
+        <dependency>


Why do cli depend on coordinator?

I fixed this issue. this is because I need to use the application object, this object I put it in common.

jerqi · 2023-08-05T17:10:48Z

conf/log4j.properties

 log4j.appender.console=org.apache.log4j.ConsoleAppender
 log4j.appender.console.Threshold=INFO
 log4j.appender.console.target=System.err
 log4j.appender.console.layout=org.apache.log4j.PatternLayout
 log4j.appender.console.layout.ConversionPattern=%d{yy/MM/dd HH:mm:ss} %p %c{1}: %m%n
-log4j.appender.RollingAppender=org.apache.log4j.RollingFileAppender


Why do we remove them?

Thank you very much for your help to review the code! I will revert this part of the change. When I was testing locally, I modified this file.

slfan1989 · 2023-08-08T10:33:28Z

@advancedxy @jerqi Can you help to review this PR again? I have made improvements based on suggestions, including adding support for pagination, appId_regex, filtering by heartBeatTime range and output in JSON format.

jerqi · 2023-08-08T10:33:47Z

You can run the command to fix style issues

mvn spotless:apply -Pspark3 -Pspark2 -Ptez -Pmr -Phadoop2.8

cli/src/main/java/org/apache/uniffle/cli/UniffleCLI.java

advancedxy · 2023-08-09T03:33:30Z

Ah, this pr is getting bigger and bigger. Let's address the existing comments here and get it merged first.

New options and refines could be done in follow up prs.

smallzhongfeng · 2023-08-09T12:03:22Z

Ah, this pr is getting bigger and bigger. Let's address the existing comments here and get it merged first.

New options and refines could be done in follow up prs.

+1, I think we should merge it at first, other features could be added slowly in the future to reduce the cost of review.

smallzhongfeng · 2023-08-09T12:08:55Z

coordinator/src/test/java/org/apache/uniffle/coordinator/GenericTestUtils.java

+    if (!result) {
+      final String exceptionErrorMsg =
+          "Timed out waiting for condition. "
+              + (org.apache.commons.lang3.StringUtils.isNotEmpty(errorMsg)


Could we import this package?

Thanks for reviewing the code, I will fix it.

smallzhongfeng

Also cc @advancedxy

advancedxy

LGTM.

It took too long, let's merge it first and do some refines in follow up prs.

advancedxy · 2023-08-11T09:21:56Z

Thanks @slfan1989 for your work. Merged.

slfan1989 · 2023-08-12T05:58:22Z

@smallzhongfeng @advancedxy Thank you very much for helping to review the code!

[apache#837] [Subtask] Display information of all application through…

0f7a1a7

… Cli.

slfan1989 marked this pull request as draft June 24, 2023 04:09

slfan1989 added 4 commits June 25, 2023 17:29

[apache#837] [Subtask] Display information of all application through…

35b2ef9

… Cli.

[apache#837] [Subtask] Display information of all application through…

5a73aa4

… Cli.

[apache#837] [Subtask] Display information of all application through…

60129a1

… Cli.

[apache#837] [Subtask] Display information of all application through…

5a0967a

… Cli.

slfan1989 marked this pull request as ready for review June 25, 2023 23:44

jerqi changed the title ~~[#837] [Subtask] Display information of all application through Cli.~~ [#837] (feat) Display information of all application through Cli. Jul 4, 2023

jerqi changed the title ~~[#837] (feat) Display information of all application through Cli.~~ [#837] feat: Display information of all application through Cli. Jul 4, 2023

jerqi requested a review from smallzhongfeng July 4, 2023 16:33

slfan1989 self-assigned this Jul 7, 2023

smallzhongfeng previously approved these changes Jul 9, 2023

View reviewed changes

Merge branch 'master' into display_apps_cli

3d8b8a3

slfan1989 dismissed smallzhongfeng’s stale review via 3d8b8a3 July 11, 2023 23:16

[apache#837] Fix CheckStyle.

87e5a32

jerqi previously approved these changes Jul 12, 2023

View reviewed changes

advancedxy mentioned this pull request Jul 13, 2023

[#999] improvement: apply REST principles in the urls of the http interfaces #1000

Merged

[apache#837] Fix CheckStyle.

3927a65

slfan1989 dismissed jerqi’s stale review via 3927a65 July 18, 2023 10:55

slfan1989 and others added 2 commits July 19, 2023 11:45

Merge branch 'master' into display_apps_cli

8e233a9

[apache#837] Fix CheckStyle.

ec0a385

[apache#846] feat(cli): Fix Junit Test.

7939489

advancedxy mentioned this pull request Jul 31, 2023

[#960][part-1] feat(dashboard): Add some dashboard interfaces. #1053

Merged

slfan1989 added 6 commits August 2, 2023 10:59

[apache#846] feat(cli): Fix Junit Test.

b58ef1e

[apache#846] feat(cli): Fix Junit Test.

8147240

[apache#846] feat(cli): Fix Junit Test.

0c86cce

[apache#846] feat(cli): Fix Junit Test.

44e58b9

[apache#846] feat(cli): Fix Junit Test.

53b84f6

[apache#846] feat(cli): Fix Junit Test.

d351571

jerqi reviewed Aug 5, 2023

View reviewed changes

slfan1989 added 4 commits August 8, 2023 13:53

[apache#846] feat(cli): Fix CheckStyle.

7b1adae

[apache#846] feat(cli): Fix CheckStyle.

ed53144

[apache#846] feat(cli): Fix CheckStyle.

1bf9a72

[apache#846] feat(cli): Fix CheckStyle.

a16a99c

slfan1989 requested review from advancedxy and jerqi August 8, 2023 10:23

[apache#846] feat(cli): Fix CheckStyle.

3d15f90

advancedxy reviewed Aug 9, 2023

View reviewed changes

cli/src/main/java/org/apache/uniffle/cli/UniffleCLI.java Outdated Show resolved Hide resolved

smallzhongfeng reviewed Aug 9, 2023

View reviewed changes

slfan1989 added 2 commits August 9, 2023 20:57

[apache#846] feat(cli): Fix CheckStyle.

bc2c87a

[apache#846] feat(cli): Fix CheckStyle.

8f90bf0

smallzhongfeng approved these changes Aug 11, 2023

View reviewed changes

advancedxy approved these changes Aug 11, 2023

View reviewed changes

advancedxy merged commit 1406e1f into apache:master Aug 11, 2023
32 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#837] feat: Display information of all application through Cli. #964

[#837] feat: Display information of all application through Cli. #964

slfan1989 commented Jun 24, 2023 •

edited by advancedxy

Loading

slfan1989 commented Jun 24, 2023

codecov-commenter commented Jun 24, 2023 •

edited

Loading

slfan1989 commented Jun 25, 2023 •

edited

Loading

advancedxy commented Jun 26, 2023

slfan1989 commented Jun 26, 2023

advancedxy commented Jun 26, 2023

slfan1989 commented Jun 27, 2023

smallzhongfeng left a comment

smallzhongfeng Jul 9, 2023

slfan1989 Jul 12, 2023

smallzhongfeng Jul 9, 2023

smallzhongfeng Jul 9, 2023

slfan1989 Jul 12, 2023

jerqi left a comment

advancedxy commented Jul 18, 2023

slfan1989 commented Jul 19, 2023

jerqi Aug 5, 2023

slfan1989 Aug 8, 2023

jerqi Aug 5, 2023

slfan1989 Aug 7, 2023

slfan1989 commented Aug 8, 2023

jerqi commented Aug 8, 2023

advancedxy commented Aug 9, 2023

smallzhongfeng commented Aug 9, 2023

smallzhongfeng Aug 9, 2023

slfan1989 Aug 9, 2023

smallzhongfeng left a comment

advancedxy left a comment

advancedxy commented Aug 11, 2023

slfan1989 commented Aug 12, 2023

[#837] feat: Display information of all application through Cli. #964

[#837] feat: Display information of all application through Cli. #964

Conversation

slfan1989 commented Jun 24, 2023 • edited by advancedxy Loading

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

slfan1989 commented Jun 24, 2023

codecov-commenter commented Jun 24, 2023 • edited Loading

Codecov Report

slfan1989 commented Jun 25, 2023 • edited Loading

advancedxy commented Jun 26, 2023

slfan1989 commented Jun 26, 2023

advancedxy commented Jun 26, 2023

slfan1989 commented Jun 27, 2023

smallzhongfeng left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jerqi left a comment

Choose a reason for hiding this comment

advancedxy commented Jul 18, 2023

slfan1989 commented Jul 19, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

slfan1989 commented Aug 8, 2023

jerqi commented Aug 8, 2023

advancedxy commented Aug 9, 2023

smallzhongfeng commented Aug 9, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smallzhongfeng left a comment

Choose a reason for hiding this comment

advancedxy left a comment

Choose a reason for hiding this comment

advancedxy commented Aug 11, 2023

slfan1989 commented Aug 12, 2023

slfan1989 commented Jun 24, 2023 •

edited by advancedxy

Loading

codecov-commenter commented Jun 24, 2023 •

edited

Loading

slfan1989 commented Jun 25, 2023 •

edited

Loading