mchades commented on code in PR #13506:
URL: https://github.com/apache/gravitino/pull/13506#discussion_r4355411023
##########
api/src/main/java/org/apache/gravitino/file/Fileset.java:
##########
@@ -71,6 +71,19 @@ public interface Fileset extends Auditable {
/** The property name for the default location name of the fileset. */
String PROPERTY_DEFAULT_LOCATION_NAME = "default-location-name";
+ /**
+ * The property name that controls whether dropping a managed fileset also
deletes its storage
+ * locations. Defaults to true; set it to false to keep the data while
removing the metadata.
+ */
+ String PROPERTY_DELETE_DATA_ON_DROP = "delete-data-on-drop";
Review Comment:
@Ystk-hsn With type conversion being added in
[#14124](https://github.com/apache/gravitino/pull/14124), I suggest narrowing
this PR to the creation guard:
- Keep the rejection of Managed filesets over existing non-empty locations,
with `allow-existing-location-as-managed=true` for explicit adoption.
- Remove `delete-data-on-drop` and its associated deletion logic, tests, and
documentation. Converting `MANAGED` to `EXTERNAL` before dropping will provide
the explicit way to retain data, keeping deletion behavior tied to the fileset
type.
Please also address the multi-location preflight comment: validate all
locations before creating any directories. Removing the retention property
would also remove the new behavior discussed in the cascade race comment, so
that issue would no longer need to be addressed within this PR.
--
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]
