jrmccluskey commented on a change in pull request #16428:
URL: https://github.com/apache/beam/pull/16428#discussion_r779019177



##########
File path: sdks/go/pkg/beam/io/filesystem/filesystem.go
##########
@@ -53,18 +57,37 @@ func New(ctx context.Context, path string) (Interface, 
error) {
 type Interface interface {

Review comment:
       I know this isn't something you established in this PR, but calling the 
interface "Interface" in Go is a little weird, it feels like it lacks 
specificity within the package (although the usage of filesystem.Interface 
elsewhere makes more sense.)

##########
File path: sdks/go/pkg/beam/io/filesystem/gcs/gcs.go
##########
@@ -139,3 +139,40 @@ func (f *fs) Size(ctx context.Context, filename string) 
(int64, error) {
 
        return attrs.Size, nil
 }
+
+// Remove the named file from the filesystem.
+func (f *fs) Remove(ctx context.Context, filename string) error {

Review comment:
       Is this filename required to be a GCS path, or can it also work on local 
paths? 

##########
File path: sdks/go/pkg/beam/io/filesystem/gcs/gcs.go
##########
@@ -139,3 +139,40 @@ func (f *fs) Size(ctx context.Context, filename string) 
(int64, error) {
 
        return attrs.Size, nil
 }
+
+// Remove the named file from the filesystem.
+func (f *fs) Remove(ctx context.Context, filename string) error {

Review comment:
       I only ask because fs.Copy() has a specific call-out for GCS paths 
whereas the rest of the functionality here does not. 




-- 
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]


Reply via email to