TheR1sing3un opened a new pull request, #7751:
URL: https://github.com/apache/paimon/pull/7751
## Purpose
PR #7746 added abstract tag stubs to `Catalog` and a complete `RESTCatalog`
implementation, but left `FileSystemCatalog` inheriting `NotImplementedError`.
The merge note said implementing filesystem tag CRUD required porting a Python
`TagManager`. As it turns out, `pypaimon/tag/tag_manager.py::TagManager` and
`FileStoreTable.create_tag` / `delete_tag` / `list_tags` / `tag_manager()`
**already exist on master**, so this PR is a thin catalog-layer wrapper that
delegates to those helpers and translates `ValueError` / return-value-`False`
into the typed catalog exceptions used by the rest of the API.
## Linked issue
Follow-up to #7746 — closes the FileSystemCatalog gap noted in that PR's
"Out of scope" section.
## Tests
- **`pypaimon/tests/filesystem_catalog_tag_test.py`** (12 new cases) —
mirrors `RESTCatalogTagCRUDTest` on the local filesystem path:
- `test_create_and_get_tag`, `test_create_tag_with_snapshot_id`
- `test_create_tag_already_exists_raises`,
`test_create_tag_already_exists_ignore`
- `test_create_tag_with_time_retained_raises_not_implemented`
(filesystem-only)
- `test_create_tag_table_not_exists`
- `test_get_tag_not_exists`
- `test_delete_tag_happy`, `test_delete_tag_not_exists`
- `test_list_tags_paged_basic` (paging via `max_results` +
`next_page_token`)
- `test_list_tags_paged_with_prefix`, `test_list_tags_paged_empty_table`
- **`pypaimon/tests/rest/rest_tag_test.py`** — removed the now-stale
`FilesystemCatalogTagInheritsNotImplementedTest` class (its assertions no
longer hold; new behavior is covered by the new file). Cleaned up unused
imports left behind.
- Regression: `pypaimon/tests/api/test_tag_dto_serde.py` (8 cases) and the
REST tag test matrix (11 cases) all still pass.
- All edited files pass `flake8 --config=dev/cfg.ini`.
## API and format
No new public Catalog method or wire format. This PR only **overrides** the
four existing abstract stubs in `FileSystemCatalog`:
| Method | Delegate / behavior |
|---|---|
| `create_tag` | `table.create_tag(tag_name, snapshot_id,
ignore_if_exists)`. `ValueError("...already exists.")` →
`TagAlreadyExistException` (only when `not ignore_if_exists`). |
| `delete_tag` | `table.delete_tag(tag_name) -> bool`. `False` →
`TagNotExistException`. |
| `get_tag` | `table.tag_manager().get(tag_name) -> Optional[Tag]`. `None` →
`TagNotExistException`. Result wrapped in `GetTagResponse` with
`snapshot=tag.trim_to_snapshot()`. |
| `list_tags_paged` | `table.list_tags() -> List[str]`. Client-side
lexicographic sort + prefix filter + `page_token` continuation cursor (no Java
equivalent of paged listing exists in `TagManager` itself). |
## Documentation
n/a — this is a parity fix for an existing public API.
## Out of scope (separate PRs)
- **`time_retained` real support** — Python's `Tag` dataclass currently
inherits only `Snapshot` fields and has no place to store `tag_create_time` /
`tag_time_retained`. This PR rejects `time_retained != None` with
`NotImplementedError` rather than silently dropping it (which would mislead
callers about TTL effects). Extending `Tag` + `TagManager.create_tag` to carry
these fields requires a coordinated change to the on-disk tag JSON format and
is tracked as a follow-up.
- **`GetTagResponse.tag_create_time`** — for the same reason, this catalog
returns `None` for `tag_create_time` and `tag_time_retained`. Cross-`FileIO`
mtime extraction is not reliable (different `FileIO` implementations expose
different metadata).
- **Branch CRUD on FileSystemCatalog** — branch CRUD requires the abstract
`rename_branch` stub and `BranchAlreadyExistException` handling introduced by
#7747 (still under review). The sister PR for FileSystemCatalog branch CRUD
will follow once #7747 lands.
- **Tag callbacks** — Java has them; Python doesn't, and they're not needed
for filesystem semantics.
## Verification
```bash
# Targeted
pytest pypaimon/tests/filesystem_catalog_tag_test.py -v
# Regression
pytest pypaimon/tests/rest/rest_tag_test.py
pypaimon/tests/api/test_tag_dto_serde.py -v
# Master-vs-fix
git stash
python -c "from pypaimon.catalog.filesystem_catalog import
FileSystemCatalog; \
print('create_tag in __dict__:', 'create_tag' in
FileSystemCatalog.__dict__)"
# Expected: False (master inherits NotImplementedError)
git stash pop
python -c "..." # Expected: True (PR adds the override)
# Lint
flake8 --config=dev/cfg.ini pypaimon/catalog/filesystem_catalog.py \
pypaimon/tests/filesystem_catalog_tag_test.py \
pypaimon/tests/rest/rest_tag_test.py
```
## Generative AI disclosure
Implementation, tests, and PR description were drafted with the assistance
of Claude (Anthropic). Claude was supplied with the existing pypaimon helpers
(`TagManager`, `FileStoreTable.create_tag` / `delete_tag` / `list_tags`,
`RESTCatalog.create_tag`'s exception mapping pattern from #7746) and the REST
tag test matrix; the design plan and code were reviewed and integrated by the
author.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
