Skip to content
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

[Improvement] Delete a table with a missing table name causes an obscure error in the Gravitino CLI #5827

Closed
justinmclean opened this issue Dec 11, 2024 · 1 comment · Fixed by #5906
Assignees
Labels
good first issue Good for newcomers improvement Improvements on everything

Comments

@justinmclean
Copy link
Member

What would you like to be improved?

When running the command gcli table delete --metalake metalake_demo --name catalog_postgres.hr you get the error Cannot create a NameIdentifier with null or empty name.

How should we improve?

Make the error more user friendly.

@justinmclean justinmclean added good first issue Good for newcomers improvement Improvements on everything labels Dec 11, 2024
@Abyss-lord
Copy link
Contributor

I would like to work on it.

Abyss-lord added a commit to Abyss-lord/gravitino that referenced this issue Dec 18, 2024
Fix cli throws an obscure error when Delete a table with a missing table name.it should give clearer hints.
Abyss-lord added a commit to Abyss-lord/gravitino that referenced this issue Dec 18, 2024
add some test case to test whether the command fuse is executed correctly
Abyss-lord added a commit to Abyss-lord/gravitino that referenced this issue Dec 18, 2024
Fix cli throws an obscure error when Delete a table with a missing table name.it should give clearer hints.
Abyss-lord added a commit to Abyss-lord/gravitino that referenced this issue Dec 18, 2024
add some test case to test whether the command fuse is executed correctly
Abyss-lord added a commit to Abyss-lord/gravitino that referenced this issue Dec 18, 2024
Fix cli throws an obscure error when Delete a table with a missing table name.it should give clearer hints.
Abyss-lord added a commit to Abyss-lord/gravitino that referenced this issue Dec 18, 2024
add some test case to test whether the command fuse is executed correctly
Abyss-lord added a commit to Abyss-lord/gravitino that referenced this issue Dec 19, 2024
Abyss-lord added a commit to Abyss-lord/gravitino that referenced this issue Dec 19, 2024
Abyss-lord added a commit to Abyss-lord/gravitino that referenced this issue Dec 19, 2024
Fix cli throws an obscure error when Delete a table with a missing table name.it should give clearer hints.
Abyss-lord added a commit to Abyss-lord/gravitino that referenced this issue Dec 19, 2024
add some test case to test whether the command fuse is executed correctly
Abyss-lord added a commit to Abyss-lord/gravitino that referenced this issue Dec 19, 2024
Abyss-lord added a commit to Abyss-lord/gravitino that referenced this issue Dec 19, 2024
Abyss-lord added a commit to Abyss-lord/gravitino that referenced this issue Dec 19, 2024
add some test case to test whether the command fuse is executed correctly
Abyss-lord added a commit to Abyss-lord/gravitino that referenced this issue Dec 19, 2024
Abyss-lord added a commit to Abyss-lord/gravitino that referenced this issue Dec 19, 2024
Abyss-lord added a commit to Abyss-lord/gravitino that referenced this issue Dec 19, 2024
Abyss-lord added a commit to Abyss-lord/gravitino that referenced this issue Dec 19, 2024
Abyss-lord added a commit to Abyss-lord/gravitino that referenced this issue Dec 19, 2024
Abyss-lord added a commit to Abyss-lord/gravitino that referenced this issue Dec 19, 2024
Abyss-lord added a commit to Abyss-lord/gravitino that referenced this issue Dec 20, 2024
Abyss-lord added a commit to Abyss-lord/gravitino that referenced this issue Dec 20, 2024
Abyss-lord added a commit to Abyss-lord/gravitino that referenced this issue Dec 21, 2024
fix ci fail and change the error message.
Abyss-lord added a commit to Abyss-lord/gravitino that referenced this issue Dec 21, 2024
add enable/disable metalake and catalog test case
Abyss-lord added a commit to Abyss-lord/gravitino that referenced this issue Dec 22, 2024
add enable/disable metalake and catalog test case
Abyss-lord added a commit to Abyss-lord/gravitino that referenced this issue Dec 22, 2024
move handling of LIST command "back" to the switch case and fix the test case.
Abyss-lord added a commit to Abyss-lord/gravitino that referenced this issue Dec 23, 2024
Abyss-lord added a commit to Abyss-lord/gravitino that referenced this issue Dec 23, 2024
Abyss-lord added a commit to Abyss-lord/gravitino that referenced this issue Dec 23, 2024
add enable/disable metalake and catalog test case
Abyss-lord added a commit to Abyss-lord/gravitino that referenced this issue Dec 23, 2024
…Delete a table with a missing table name (apache#5906)

### What changes were proposed in this pull request?

Fix CLI throws an obscure error when Delete a table with a missing table
name.it should give clearer hints.

### Why are the changes needed?

Fix: apache#5827 

### Does this PR introduce _any_ user-facing change?

NO

### How was this patch tested?

#### local bash test

```bash
bin/gcli.sh table delete --metalake demo_metalake
Missing required argument(s): catalog, schema, table

bin/gcli.sh table delete --metalake demo_metalake --name Hive_catalog
# output: Missing required argument(s): schema, table

bin/gcli.sh table delete --metalake demo_metalake --name Hive_catalog.default
# output: Missing required argument(s): table

bin/gcli.sh table details --metalake demo_metalake --name Hive_catalog.default
# output
# Malformed entity name.
# Missing required argument(s): table
```

#### Unit test

add some test case to test whether the command fuse is executed
correctly.
Abyss-lord added a commit to Abyss-lord/gravitino that referenced this issue Dec 29, 2024
…Delete a table with a missing table name (apache#5906)

### What changes were proposed in this pull request?

Fix CLI throws an obscure error when Delete a table with a missing table
name.it should give clearer hints.

### Why are the changes needed?

Fix: apache#5827 

### Does this PR introduce _any_ user-facing change?

NO

### How was this patch tested?

#### local bash test

```bash
bin/gcli.sh table delete --metalake demo_metalake
Missing required argument(s): catalog, schema, table

bin/gcli.sh table delete --metalake demo_metalake --name Hive_catalog
# output: Missing required argument(s): schema, table

bin/gcli.sh table delete --metalake demo_metalake --name Hive_catalog.default
# output: Missing required argument(s): table

bin/gcli.sh table details --metalake demo_metalake --name Hive_catalog.default
# output
# Malformed entity name.
# Missing required argument(s): table
```

#### Unit test

add some test case to test whether the command fuse is executed
correctly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers improvement Improvements on everything
Projects
None yet
2 participants