amogh-jahagirdar commented on code in PR #11565:
URL: https://github.com/apache/iceberg/pull/11565#discussion_r1878222555
##########
gcp/src/main/java/org/apache/iceberg/gcp/gcs/GCSFileIO.java:
##########
@@ -242,4 +248,106 @@ private void internalDeleteFiles(Stream<BlobId>
blobIdsToDelete) {
Streams.stream(Iterators.partition(blobIdsToDelete.iterator(),
gcpProperties.deleteBatchSize()))
.forEach(batch -> client().delete(batch));
}
+
+ @Override
+ public boolean recoverFile(String path) {
+ Preconditions.checkArgument(
+ !Strings.isNullOrEmpty(path), "Cannot recover file: path must not be
null or empty");
+
+ try {
+ BlobId blobId = BlobId.fromGsUtilUri(path);
+
+ // first attempt to restore with soft-delete
+ if (recoverSoftDeletedObject(blobId)) {
+ return true;
+ }
+
+ // fallback to restoring by copying the latest version
+ if (recoverLatestVersion(blobId)) {
+ return true;
+ }
+
+ } catch (IllegalArgumentException e) {
+ LOG.warn("Invalid GCS path format: {}", path, e);
+ }
Review Comment:
Actually just noticed this, not sure it makes sense to swallow the
IllegalArgumentException in case the path format is incorrect? I consider this
just like the null or empty case where `recoverFile` can just throw. The best
effort really is for the calls to attempt recovery, not the other validation
imo.
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
