-
Notifications
You must be signed in to change notification settings - Fork 8.7k
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
YARN-11506.The formatted yarn queue list is displayed on CLI #5716
Conversation
🎊 +1 overall
This message was automatically generated. |
<dependency> | ||
<groupId>com.blinkfox</groupId> | ||
<artifactId>mini-table</artifactId> | ||
<version>1.0.0</version> |
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.
This version needs to be added in the parent project, We need to make sure there are no CVE issues.
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.
Thanks for the advice. I'll check it out
567eda2
to
493e7fa
Compare
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
public FormattingCLIUtils addDatas(Object... objects) { | ||
return this.appendRows(TableRowType.DATA, objects); | ||
} | ||
private FormattingCLIUtils appendRows(TableRowType tableRowType, Object... objects) { |
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.
Can we add some comments to the method?
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.
It has been modified, please review it again
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
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.
minor stuff,
rest can you change "datas" everywhere to "data""
Data itself plural
https://www.britannica.com/dictionary/eb/qa/Is-Data-Singular-or-Plural-#:~:text=Technically%2C%20%22data%22%20is%20a,in%20visitors%20to%20state%20parks.
public FormattingCLIUtils addHeaders(Object... objects) { | ||
return this.appendRows(TableRowType.HEADER, objects); | ||
} |
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.
where is this being used?
/** | ||
* Puts a string in the middle of a given size. | ||
* @param str | ||
* @param size | ||
* @param padChar | ||
* @return | ||
*/ |
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.
fill these values for javadoc
public FormattingCLIUtils addDatas(List<?> datas) { | ||
return this.appendRows(TableRowType.DATA, datas.toArray()); |
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.
unused??
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
@ayushtkn I have modified the problem you mentioned, please help me review it |
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.
typos, rest LGTM
* A way to build rows of data. | ||
* @param datas dataLine | ||
*/ | ||
private void buildRowLine(List<String> datas) { |
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.
nit
change to data
*/ | ||
private static class TableRow { | ||
private TableRowType tableRowType; | ||
private List<String> datas; |
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.
nit
change to data
private void buildTable() { | ||
this.buildTitle(); | ||
for (int i = 0, len = this.tableRows.size(); i < len; i++) { | ||
List<String> datas = this.tableRows.get(i).datas; |
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.
data
🎊 +1 overall
This message was automatically generated. |
@slfan1989 @ayushtkn Thank you both for your review |
YARN-11506.The formatted yarn queue list is displayed on the command line
Original presentation format:
4 queues were found :
Queue Name Queue Path State Capacity Current Capacity Maximum Capacity Weight Maximum Parallel Apps
queueA root.queueA RUNNING 40.00% 50.00% 80.00% -1.00 10
queueB root.queueB RUNNING 40.00% 50.00% 80.00% -1.00 10
queueC root.queueC RUNNING 40.00% 50.00% 80.00% -1.00 10
queueD root.queueD RUNNING 40.00% 50.00% 80.00% -1.00 10
New way of presentation:
+----------------------------------------------------------------------------------------------------------------------+
| 4 queues were found |
+------------+-------------+---------+----------+------------------+------------------+--------+-----------------------+
| Queue Name | Queue Path | State | Capacity | Current Capacity | Maximum Capacity | Weight | Maximum Parallel Apps |
+------------+-------------+---------+----------+------------------+------------------+--------+-----------------------+
| queueA | root.queueA | RUNNING | 40.00% | 50.00% | 80.00% | -1.00 | 10 |
| queueB | root.queueB | RUNNING | 40.00% | 50.00% | 80.00% | -1.00 | 10 |
| queueC | root.queueC | RUNNING | 40.00% | 50.00% | 80.00% | -1.00 | 10 |
| queueD | root.queueD | RUNNING | 40.00% | 50.00% | 80.00% | -1.00 | 10 |
+------------+-------------+---------+----------+------------------+------------------+--------+-----------------------+