[#14123] feat(fileset): Support managed and external type conversion by mchades · Pull Request #14124 · apache/gravitino

mchades · 2026-10-22T02:37:48Z

What changes were proposed in this pull request?

Add FilesetChange.updateType, Python update_type, and REST updateType support for changing existing filesets between MANAGED and EXTERNAL. Conversion preserves the fileset ID, locations, and files. Update the OpenAPI specification and usage documentation.

Also correct the quick-start wording that an existing path automatically makes a fileset external. This is a documentation correction; creation behavior is unchanged.

Why are the changes needed?

Users need to change an existing fileset's lifecycle ownership without dropping and recreating it. Converting to external allows its registration to be dropped while retaining the data; converting to managed explicitly accepts deletion on drop.

Related: #12550 and #13506.

Fix: #14123

Does this PR introduce any user-facing change?

Yes. A new fileset alter operation in Java, Python, and REST. Conversion does not move, create, or delete files; subsequent direct and schema-cascade drops use the updated type.

How was this patch tested?

280 targeted Java tests passed: TestFilesetChange, TestFilesetUpdatesRequest, TestFilesetCatalog, TestFilesetOperations, and TestFilesetCatalogOperations with -PskipITs.
3 Python tests passed: python -m unittest tests.unittests.test_fileset_type_change tests.unittests.test_fileset_catalog_api.
./gradlew :docs:build, Spotless, Black, Pylint, and git diff --check passed.
Added coverage for both directions, preserved identity and data, multiple locations, repeated conversion, batched changes, direct drop, and schema cascade.

Docker integration tests were not run.

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

3 open issues

This doc change updates create-time semantics (“Pointing at an existing path does not… · New
The updated class-level Javadoc is grammatically ambiguous (“update the comment or type of a… · New
This docstring now reads ambiguously (“update the comment or type of a fileset”) and is split in a… · New

What changed in this PR

Adds a new “update fileset type” alter operation across Java, Python, and REST, enabling conversion between MANAGED and EXTERNAL without changing a fileset’s identity, locations, or existing files.

Changes:

Introduces FilesetChange.updateType / update_type and corresponding DTO/request plumbing for REST.
Implements catalog-side application of type changes and adds validation for invalid updateType requests.
Updates OpenAPI + docs and adds test coverage across server, API, catalog, and Python/Java clients.

File	Description
server/src/test/java/org/apache/gravitino/server/web/rest/TestFilesetOperations.java	Adds REST-layer tests for updateType and invalid request handling.
docs/open-api/filesets.yaml	Extends OpenAPI schema with `UpdateFilesetTypeRequest`.
docs/manage-fileset-metadata-using-gravitino.md	Documents the new alter operation and provides examples.
docs/filesets.md	Updates lifecycle semantics wording and links to the new conversion operation.
common/src/test/java/org/apache/gravitino/dto/requests/TestFilesetUpdatesRequest.java	Adds serialization/validation tests for the new update request.
common/src/main/java/org/apache/gravitino/dto/requests/FilesetUpdateRequest.java	Adds `UpdateFilesetTypeRequest` DTO and validation.
clients/client-python/tests/unittests/test_fileset_type_change.py	Adds unit tests for Python `update_type` and request validation.
clients/client-python/gravitino/dto/requests/fileset_update_request.py	Adds Python DTO for `updateType`.
clients/client-python/gravitino/client/fileset_catalog.py	Maps `FilesetChange.UpdateFilesetType` to DTO requests.
clients/client-python/gravitino/api/file/fileset_change.py	Adds Python `FilesetChange.update_type` and `UpdateFilesetType`.
clients/client-java/src/test/java/org/apache/gravitino/client/TestFilesetCatalog.java	Adds client test for alterFileset with updateType change.
clients/client-java/src/main/java/org/apache/gravitino/client/DTOConverters.java	Converts `FilesetChange.UpdateFilesetType` to REST update request.
catalogs/catalog-fileset/src/test/java/org/apache/gravitino/catalog/fileset/TestFilesetCatalogOperations.java	Adds integration-style tests for preservation semantics and drop behavior.
catalogs/catalog-fileset/src/main/java/org/apache/gravitino/catalog/fileset/FilesetCatalogOperations.java	Applies UpdateFilesetType when updating persisted entity.
api/src/test/java/org/apache/gravitino/file/TestFilesetChange.java	Adds API-level unit tests for `FilesetChange.updateType`.
api/src/main/java/org/apache/gravitino/file/FilesetChange.java	Introduces `updateType` and `UpdateFilesetType` change type.

_{Review effort: Lite}

mchades · 2026-10-22T03:36:57Z

+**2. Create the fileset.** Give it a name, a type, and a storage location. Choose `MANAGED` to let
+Gravitino manage the data lifecycle, or `EXTERNAL` to retain the data when the fileset is dropped.
+Pointing at an existing path does not automatically make a fileset external.


Creation behavior is unchanged. The previous quick-start wording incorrectly implied that an existing path makes a fileset external. I’ll clarify in the PR description that this is a documentation correction.

mchades · 2026-10-22T03:48:50Z

 /**
 * A fileset change is a change to a fileset. It can be used to rename a fileset, update the comment
- * of a fileset, set a property and value pair for a fileset, or remove a property from a fileset.
+ * or type of a fileset, set a property and value pair for a fileset, or remove a property from a
+ * fileset.


I’ve listed comment updates and type updates separately.

mchades · 2026-10-22T03:48:59Z

 class FilesetChange(ABC):
    """A fileset change is a change to a fileset. It can be used to rename a fileset, update the comment
-    of a fileset, set a property and value pair for a fileset, or remove a property from a fileset.
+    or type of a fileset, set a property and value pair for a fileset, or remove a property from a fileset.
    """


I’ve listed the operations separately and kept the wording consistent with the Java API.

jerryshao

Thanks for the PR. The conversion itself looks correct: id, locations and files are preserved, and subsequent drop / schema cascade follow the new type, including multi-location filesets. The main concern is authorization for EXTERNAL -> MANAGED (see inline comment; the auth check lives in FilesetOperations.java:290, outside the diff). A couple of doc clarifications and a Python nit as well.

jerryshao · 2026-10-22T07:40:19Z

        newName = ((FilesetChange.RenameFileset) change).getNewName();
      } else if (change instanceof FilesetChange.UpdateFilesetComment) {
        newComment = ((FilesetChange.UpdateFilesetComment) change).getNewComment();
+      } else if (change instanceof FilesetChange.UpdateFilesetType) {


updateType is authorized as a normal alterFileset (FILESET::OWNER or ANY_WRITE_FILESET, see FilesetOperations.java:290), while drop is owner-only. A principal with write but not drop can convert EXTERNAL to MANAGED and thereby make a later owner drop or schema cascade recursively delete every storage location. That is a new way for write to cause data loss. Please require the drop privilege for EXTERNAL to MANAGED, or document and test that write is intentionally enough to accept deletion.

I kept type conversion under the existing alterFileset authorization, consistent with WRITE_FILESET allowing data writes and metadata changes. I documented this and added an authorization test showing that a writer can convert both ways but still cannot drop the fileset.

jerryshao · 2026-10-22T07:40:19Z

+A managed fileset belongs to Gravitino. Creating it creates the directory if needed, and deleting
+it deletes the data.

 An external fileset points at a location that already exists and stays under someone else's control.


The quick-start correction is right: an existing path does not make a fileset external. The following sentence still says an external fileset points at a location that already exists. Create mkdirs for both types when the path is missing and adopts an existing directory for both. Please describe type as an explicit lifecycle flag, not as "directory already existed."

I updated the definition of EXTERNAL to focus on data retention and clarified that either type creates a missing directory when filesystem operations are enabled.

jerryshao · 2026-10-22T07:40:19Z

+its identifier, name, storage locations, properties, and existing files are preserved. Setting the
+current type is allowed.
+
+Change a `MANAGED` fileset to `EXTERNAL` before dropping it to retain its files. Changing an


Conversion to MANAGED deletes locations on drop only when filesystem ops are enabled. With disable-filesystem-ops=true, drop still removes only metadata. Worth one sentence next to the new lifecycle text so "accepts deletion" is not read as unconditional.

I clarified that managed data deletion requires filesystem operations to be enabled. With disable-filesystem-ops=true, drop removes only metadata.

jerryshao · 2026-10-22T07:40:19Z

+    class UpdateFilesetType:
+        """A fileset change to update the type without changing data or locations."""
+
+        _new_type: Fileset.Type = field(metadata=config(field_name="newType"))


nit: UpdateFilesetType works, but please match the other change classes: snake_case field metadata on the change object (keep newType on the request DTO) and an explicit __eq__.

I switched the change object’s field metadata to new_type and added an explicit __eq__ to match the other change classes. The request DTO still uses newType.

…ntion Validate all location filesystem states before creating directories. Preserve nested new locations and avoid leaving new directories when a later location is rejected. Remove delete-data-on-drop in favor of the separate type-conversion work in apache#14124. Retain the creation guard and explicit adoption option, with regression coverage and updated documentation.

feat(fileset): Support managed and external type conversion

3715b00

Copilot AI balanced review requested due to automatic review settings October 22, 2026 02:37

Copilot AI reviewed Oct 22, 2026

View reviewed changes

docs(fileset): Clarify supported fileset change operations

04f758a

Copilot stopped reviewing on behalf of mchades due to an error October 22, 2026 04:44
The job was not acquired by a runner even after multiple attempts.

jerryshao reviewed Oct 22, 2026

View reviewed changes

fix(fileset): Clarify type conversion permissions and lifecycle

3ead2ae

mchades requested a review from jerryshao October 22, 2026 14:03

mchades closed this Oct 22, 2026

mchades reopened this Oct 22, 2026

Merge branch 'main' into codex/fileset-type-conversion

461978a

jerryshao approved these changes Oct 23, 2026

View reviewed changes

Merge branch 'main' into codex/fileset-type-conversion

786d7d0

mchades mentioned this pull request Oct 23, 2026

[#12550] improvement(fileset): Guard managed fileset creation on non-empty locations #13506

Open

jerryshao merged commit 2fe1b28 into apache:main Oct 23, 2026
4 checks passed

github-actions Bot assigned mchades Oct 23, 2026

mchades deleted the codex/fileset-type-conversion branch October 24, 2026 14:54

Conversation

mchades commented Oct 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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?

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jerryshao left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mchades commented Oct 22, 2026 •

edited

Loading