Add --purge option to drop table CLI - #3718
Conversation
jayceslesar
left a comment
There was a problem hiding this comment.
Does it make more sense for this to be its own CLI command instead of a flag? Doesn't make a huge difference to me but I think the option you have here more closely aligns with Java DDL
|
|
||
| @drop.command() | ||
| @click.argument("identifier") | ||
| @click.option("--purge", is_flag=True, help="Physically delete all table files.") |
There was a problem hiding this comment.
Can we update cli.md? If not, PyIceberg users might not be aware of this option.
There was a problem hiding this comment.
@ebyhr Good point ! Added short example to cli.md !
There was a problem hiding this comment.
nice, i wonder if in the future we could use something like https://github.com/mkdocs/mkdocs-click?
not in this PR but maybe worth investigating!
There was a problem hiding this comment.
nit: can we set this default to False to not break the existing behavior?
kevinjqliu
left a comment
There was a problem hiding this comment.
thanks for adding this!
| Dropped table: nyc.taxis | ||
| ``` | ||
|
|
||
| To also purge the table files through the configured catalog, pass `--purge`: |
There was a problem hiding this comment.
| To also purge the table files through the configured catalog, pass `--purge`: | |
| To request purge for the underlying table files through the configured catalog, pass `--purge`: |
the purge option is a request to delete, the catalog might not honor it
maybe something like this
|
|
||
| @drop.command() | ||
| @click.argument("identifier") | ||
| @click.option("--purge", is_flag=True, help="Physically delete all table files.") |
There was a problem hiding this comment.
nit: can we set this default to False to not break the existing behavior?
Rationale for this change
PyIceberg already exposes
Catalog.purge_table, but the CLI only supports dropping a table throughcatalog.drop_table.This adds a
--purgeoption topyiceberg drop tableso users can request the existing catalog purge behavior from the CLI. The default behavior remains unchanged: without--purge, the command still callscatalog.drop_table.Are these changes tested?
Yes.
make test PYTEST_ARGS="tests/cli/test_console.py -v -k 'drop_table'"uv run prek run ruff --files pyiceberg/cli/console.py tests/cli/test_console.pyuv run prek run ruff-format --files pyiceberg/cli/console.py tests/cli/ test_console.pyAre there any user-facing changes?
Yes.
pyiceberg drop table <identifier> --purgeis now available and callscatalog.purge_table(identifier).