[#5746] improve(CLI): Support table format output for Audit command #6461

Abyss-lord · 2025-02-15T16:07:41Z

What changes were proposed in this pull request?

Support table format output for Audit command
Refactor the output format.

Why are the changes needed?

Fix: #5746

Does this PR introduce any user-facing change?

No

How was this patch tested?

local test + ut

gcli metalake list -i  --output table
+-------------------+
|     METALAKE      |
+-------------------+
| demo              |
| cli_demo          |
| demo_metalake     |
| test_cli_metalake |
| tyy               |
| demo3             |
+-------------------+

gcli metalake details -i --output table  -m demo_metalake
+---------------+-------------+
|   METALAKE    |   COMMENT   |
+---------------+-------------+
| demo_metalake | new comment |
+---------------+-------------+

gcli catalog  list -i --output table  -m demo_metalake
+-------------------+
|      CATALOG      |
+-------------------+
| File              |
| Hive_catalog      |
| Iceberg_catalog   |
| Mysql_catalog     |
| Test_hive_catalog |
+-------------------+

gcli catalog  details --name Hive_catalog  -i --output table  -m demo_metalake
+--------------+------------+----------+-------------+
|   CATALOG    |    TYPE    | PROVIDER |   COMMENT   |
+--------------+------------+----------+-------------+
| Hive_catalog | RELATIONAL | hive     | new comment |
+--------------+------------+----------+-------------+

gcli schema list --name Hive_catalog  -i --output table  -m demo_metalake
+---------------------------+
|          SCHEMA           |
+---------------------------+
| convert_example           |
| default                   |
| empty                     |
| hive_best_performance     |
| hive_function_test        |
| hive_spark_diff           |
| hive_to_spark             |
| smoke_testccdp_hive       |
| test_audit                |
| test_carbondata           |
| test_drop                 |
| test_gravitino_hive       |
| test_gravitino_iceberg    |
| test_hive_convert_example |
| test_multiple_data_source |
| test_pancx                |
+---------------------------+

gcli schema details --name Hive_catalog.default  -i --output table  -m demo_metalake
+---------+-----------------------+
| SCHEMA  |        COMMENT        |
+---------+-----------------------+
| default | Default Hive database |
+---------+-----------------------+

gcli table list --name Hive_catalog.default  -i --output table  -m demo_metalake
+---------------------------------------+
|                 NAME                  |
+---------------------------------------+
| test_create_txt_tbl_without_delimited |
| test_create_regex_tbl                 |
| test_alter_command                    |
| test_dates                            |
| test_all_types_table                  |
| test_complex_sales                    |
| test_complex_employees                |
| test_complex_type                     |
| test_jinjia_tbl2                      |
| test                                  |
| test1                                 |
| example_table                         |
| example_table2                        |
| test_privilege                        |
+---------------------------------------+

gcli table details --name Hive_catalog.default.test_dates  -i --output table  -m demo_metalake
+-----------------+-----------+---------+
|      NAME       |   TYPE    | COMMENT |
+-----------------+-----------+---------+
| id              | integer   | null    |
| event_date      | date      | null    |
| event_timestamp | timestamp | null    |
+-----------------+-----------+---------+


gcli metalake  details   --output table  -m demo_metalake --audit  -i
+-----------+--------------------------+-----------+--------------------------+
|  creator  |       create_time        | modified  |       modify_time        |
+-----------+--------------------------+-----------+--------------------------+
| anonymous | 2024-12-04T07:41:18.512Z | anonymous | 2025-01-14T07:56:25.496Z |
+-----------+--------------------------+-----------+--------------------------+

gcli catalog  details --name Hive_catalog  -i --output table  -m demo_metalake --audit
+-----------+--------------------------+-----------+--------------------------+
|  creator  |       create_time        | modified  |       modify_time        |
+-----------+--------------------------+-----------+--------------------------+
| anonymous | 2024-12-05T01:20:40.512Z | anonymous | 2024-12-27T02:33:24.968Z |
+-----------+--------------------------+-----------+--------------------------+

gcli schema  details --name Hive_catalog.default --output table  -m demo_metalake --audit -i
+---------+-------------+----------+-------------+
| creator | create_time | modified | modify_time |
+---------+-------------+----------+-------------+
| public  | null        | null     | null        |
+---------+-------------+----------+-------------+

gcli table  details --name Hive_catalog.default.test_dates --output table  -m demo_metalake --audit -i
+-----------+----------------------+----------+-------------+
|  creator  |     create_time      | modified | modify_time |
+-----------+----------------------+----------+-------------+
| panchenxi | 2024-07-24T07:20:52Z | null     | null        |
+-----------+----------------------+----------+-------------+

…mand Support table format output for Audit command.

…mand format the code.

Abyss-lord · 2025-02-15T16:14:24Z

@justinmclean @waukin , could you please review this PR when you have time? I’d really appreciate your feedback.

…mand Add Table format.

… server Rename the columns.

tengqm

This PR is a mixture of several changes.
It is about a refactor of the table output format, a beautified version of the table renderer, a change to the audit command and several other things.
Please try your best to make a PR self-contained, atomic.

clients/cli/src/main/java/org/apache/gravitino/cli/commands/ListTables.java

clients/cli/src/main/java/org/apache/gravitino/cli/outputs/BaseOutputFormat.java

tengqm · 2025-02-17T02:43:24Z

clients/cli/src/main/java/org/apache/gravitino/cli/outputs/BaseOutputFormat.java

+  public BaseOutputFormat(CommandContext context) {
+    Preconditions.checkNotNull(context, "CommandContext cannot be null");
+    this.context = context;
+    this.limit = context.outputLimit();


I'm not sure this limit is gonna be a generic parameter for most commands.

I am worried that too many schemas or tables will affect the display

clients/cli/src/main/java/org/apache/gravitino/cli/outputs/BaseOutputFormat.java

tengqm · 2025-02-17T02:45:56Z

clients/cli/src/main/java/org/apache/gravitino/cli/outputs/Column.java

+ * Represents a column in a formatted table output. Manages column properties including header,
+ * alignment, and content cells. Handles width calculations.
+ */
+public class Column {


This class can be spit into a separate PR.

tengqm · 2025-02-17T02:47:33Z

clients/cli/src/main/java/org/apache/gravitino/cli/outputs/Constant.java

+
+import com.google.common.collect.ImmutableList;
+
+public class Constant {


I'm not sure we want to beautify the table output to this extent.
It is meaningless, at least to me.

Instead of changing to a pretty tabular output, it's easier to unit test the output, and instead of writing it directly to std out, this implementation can now unit test the output with a simple mock.

tengqm · 2025-02-17T02:48:34Z

clients/cli/src/main/java/org/apache/gravitino/cli/outputs/PlainFormat.java

+   * @throws IllegalArgumentException if the object type is not supported
+   */
+  public static void output(Object entity, CommandContext context) {
+    if (entity instanceof Metalake) {


Please change this long chain of if...else... into a switch ?

@tengqm JDK8 does not support pattern matching. So we can't use switch-case overwriting.

clients/cli/src/main/java/org/apache/gravitino/cli/outputs/PlainFormat.java

…eOutputFormat.java Co-authored-by: Qiming Teng <[email protected]>

Abyss-lord · 2025-02-19T03:37:16Z

@justinmclean should I and split this pr into several issues?

… roles is bound to many metadata. (apache#6455) ### What changes were proposed in this pull request? fix issue apache#6238 improve performance when a single role is bound to many metadata. ### Why are the changes needed? Use batch queries when getting role securable object full names instead of loop queries to get each securable object full name. Fix: apache#6238 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Unit tests and integration tests have all passed, this feature has been running internally at Xiaomi for two weeks. Co-authored-by: luoxin5 <[email protected]>

…o configure gvfs-fuse. (apache#6465) ### What changes were proposed in this pull request? Fix the bug of using a relative path to configure gvfs-fuse. ### Why are the changes needed? Fix: apache#6464 ### Does this PR introduce _any_ user-facing change? NO ### How was this patch tested? Manually test

…FSFileSystemProvider` problem (apache#6463) ### What changes were proposed in this pull request? In the current code base, we need to add `catalog-hadoop` to make GVFS client works or class `HDFSFileSystemProvider` and `LocalFileSystemProvider` can't be found. ### Why are the changes needed? It's a bug. Fix: apache#6462 ### Does this PR introduce _any_ user-facing change? N/A ### How was this patch tested? ITs, UTs and test locally.

### What changes were proposed in this pull request? This pr removes outdated tips from `web-ui.md` because, as indicated in the subsequent text, operations such as creating or modifying schemas, tables, or filesets can now be performed through the web UI. ### Why are the changes needed? Remove outdated tips from `web-ui.md` ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? manual review

…on parameters through environment variables (apache#6458) ### What changes were proposed in this pull request? Add the environment variables `GRAVITINO_JDBC_USER`, `GRAVITINO_JDBC_PASSWORD`, and `GRAVITINO_S3_ENDPOINT` to the `rewrite_config.py`, so database user and password can be set by the user in a container environment as well as a custom S3 endpoint. ### Why are the changes needed? Currently, JDBC user and PW are hardcoded to `iceberg` and cannot be changed when using the container images. One workaround is including the values in the JDBC connection string, but this neither elegant nor in line with how containers commonly handle this issue. Additionally, it wasn't possible to set the value of `gravitino.iceberg-rest.s3-endpoint` in a container environment, which can be useful in a variety of situations when using a provider other than AWS. ### Does this PR introduce _any_ user-facing change? Added the three aforementioned two environment variables that can be used by the user. ### How was this patch tested? Was tested locally only, since this is a minimal change.

…mand Support table format output for Audit command.

…mand fix some bugs.

justinmclean · 2025-02-19T04:57:21Z

clients/cli/src/main/java/org/apache/gravitino/cli/CommandContext.java

@@ -32,6 +32,8 @@ public class CommandContext {
  private final boolean quiet;
  private final CommandLine line;
  private final String auth;
+  // TODO make it final
+  private int outputLimit;


can it be made final?

justinmclean · 2025-02-19T04:58:05Z

clients/cli/src/main/java/org/apache/gravitino/cli/CommandContext.java

@@ -55,6 +57,7 @@ public CommandContext(CommandLine line) {
            ? line.getOptionValue(GravitinoOptions.OUTPUT)
            : Command.OUTPUT_FORMAT_PLAIN;
    this.quiet = line.hasOption(GravitinoOptions.QUIET);
+    this.outputLimit = -1;


It's unclear to me what this is for - perhaps it needs a better name or better still in another PR

justinmclean · 2025-02-19T05:05:43Z

clients/cli/src/test/java/org/apache/gravitino/cli/integration/test/TableFormatOutputIT.java

@@ -106,7 +106,7 @@ public void testMetalakeListCommand() {
    String output = new String(outputStream.toByteArray(), StandardCharsets.UTF_8).trim();
    assertEquals(
        "+-------------+\n"
-            + "| metalake    |\n"
+            + "|  METALAKE   |\n"


Please don't change the headings to all capitals it makes it harder to read

justinmclean · 2025-02-19T05:06:41Z

Yep I think multiple PRs would be better as this one PR is trying to do too much

Abyss-lord and others added 3 commits February 16, 2025 00:05

[apache#5746] improve(CLI): Support table format output for Audit com…

176eeff

…mand Support table format output for Audit command.

Merge branch 'main' into feat-table-output

8d8ed0a

[apache#5746] improve(CLI): Support table format output for Audit com…

128cafe

…mand format the code.

Abyss-lord added 2 commits February 16, 2025 11:53

[apache#5746] improve(CLI): Support table format output for Audit com…

663c338

…mand Add Table format.

[apache#6067] improvement(core): Support model pre event to Gravitino…

fa90f4f

… server Rename the columns.

tengqm reviewed Feb 17, 2025

View reviewed changes

Update clients/cli/src/main/java/org/apache/gravitino/cli/outputs/Bas…

6aa2b39

…eOutputFormat.java Co-authored-by: Qiming Teng <[email protected]>

FourFriends and others added 8 commits February 19, 2025 11:48

[apache#5746] improve(CLI): Support table format output for Audit com…

2985aff

…mand Support table format output for Audit command.

[apache#5746] improve(CLI): Support table format output for Audit com…

950cf34

…mand fix some bugs.

Merge branch 'main' into feat-table-output

98c3f43

justinmclean reviewed Feb 19, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#5746] improve(CLI): Support table format output for Audit command #6461

[#5746] improve(CLI): Support table format output for Audit command #6461

Abyss-lord commented Feb 15, 2025 •

edited

Loading

Abyss-lord commented Feb 15, 2025

tengqm left a comment

tengqm Feb 17, 2025

Abyss-lord Feb 19, 2025

tengqm Feb 17, 2025

tengqm Feb 17, 2025

Abyss-lord Feb 19, 2025

tengqm Feb 17, 2025

Abyss-lord Feb 19, 2025

Abyss-lord commented Feb 19, 2025

justinmclean Feb 19, 2025

justinmclean Feb 19, 2025

justinmclean Feb 19, 2025

justinmclean commented Feb 19, 2025


		import com.google.common.collect.ImmutableList;

		public class Constant {

[#5746] improve(CLI): Support table format output for Audit command #6461

Are you sure you want to change the base?

[#5746] improve(CLI): Support table format output for Audit command #6461

Conversation

Abyss-lord commented Feb 15, 2025 • edited 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?

Abyss-lord commented Feb 15, 2025

tengqm 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Abyss-lord commented Feb 19, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

justinmclean commented Feb 19, 2025

Abyss-lord commented Feb 15, 2025 •

edited

Loading