lostluck commented on code in PR #31783:
URL: https://github.com/apache/beam/pull/31783#discussion_r1675856352
##########
sdks/go/pkg/beam/util/gcsx/gcs.go:
##########
@@ -63,6 +64,41 @@ func Upload(ctx context.Context, client *storage.Client,
project, bucket, object
}
+// SoftDeletePolicyEnabled checks whether soft delete policy is enabled.
+// Note that soft delete policy is disabled when RetentionDuration is zero.
Otherwise,
+// soft delete policy is enabled by default.
+func SoftDeletePolicyEnabled(attrs *storage.BucketAttrs) bool {
Review Comment:
Consider whether this function (and the other one being added) needs to be
exported. Are we going to call this outside of this package? Are users ever
expected to call it themselves?
If there's no need for it to be on the public user surface, then it
shouldn't be exported. (Package level names with an initial Capital letter
indicates exported or not, in Go).
Note you can always call unexported functions within the test files for the
same package.
##########
sdks/go/pkg/beam/util/gcsx/gcs.go:
##########
@@ -63,6 +64,41 @@ func Upload(ctx context.Context, client *storage.Client,
project, bucket, object
}
+// SoftDeletePolicyEnabled checks whether soft delete policy is enabled.
+// Note that soft delete policy is disabled when RetentionDuration is zero.
Otherwise,
+// soft delete policy is enabled by default.
+func SoftDeletePolicyEnabled(attrs *storage.BucketAttrs) bool {
+ if attrs == nil || attrs.SoftDeletePolicy == nil {
+ return false
+ }
+ return attrs.SoftDeletePolicy.RetentionDuration > 0
+}
+
+// WarnIfSoftDeletePolicyEnabled warns if the soft delete policy is enabled
for the temporary bucket.
+func WarnIfSoftDeletePolicyEnabled(ctx context.Context, client
*storage.Client, bucket string) {
+ exists, err := BucketExists(ctx, client, bucket)
+ if err != nil {
+ panic("Fail to check BucketExists.")
Review Comment:
It's not a good idea to panic deep into libraries. Consider returning an
error instead or simply silently return.
In this case, it's unexpected to panic if it's called on a non existent
bucket, when the function's goal is to log a message.
Return an error perhaps,but certainly not crash the program.
--
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]
