-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat: add GCS support as file provider (#2013) #2435
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: development
Are you sure you want to change the base?
Conversation
…dsa/gofr into feat/gcs-file-provider
…ions and fix linting issue
…ions and fix linting issue
…ntation and removing separate logger for gcs
|
@Suryakantdsa I have updated this PR with the Seek, ReadAt and WriteAt method implemented by you. I think we have refactored almost all of the review comments given on PR #2013. And moreover GCS implementation is now using these common components. (logger, metrics). @akshat-kumar-singhal Can you take a final review on this. |
|
Thank you so much @Umang01-hash for taking the time to review and implement the required GCS changes. Really appreciate your help! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are in a good state structurally - some improvements have been shared. Summarising:
- Common capabilities should not be present in the file provider (gcs) package
- Establish consistency in the logging pattern in terms of log level and avoid redundancy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most of the code here can be promoted to the file package as it's a common implementation, agnostic to the file provider
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have moved common utility functions (GetBucketName, GetObjectName to file/fs.go since they're provider-agnostic.
However, the actual directory operation implementations (Mkdir, ReadDir, Stat, etc.) remain in gcs/fs_dir.go because they contain GCS-specific logic (client calls, error handling, observability with GCS context).
…s operations to file directory
…for_file_systems' into fix/logger_metrics_for_file_systems
|
Further in this PR I have made the follwing changes: We’ve introduced a new To streamline file and directory operations, we’ve added two key components:
Both layers delegate actual storage actions to the StorageProvider interface, which defines: // StorageProvider is the unified interface for all cloud storage providers (GCS, S3, FTP, SFTP).
// It abstracts low-level storage operations, allowing common implementations for directory operations,
// metadata handling, and observability.
//
// All providers (GCS, S3, FTP, SFTP) implement this interface to integrate with GoFr's file system.
type StorageProvider interface {
Connect(ctx context.Context) error
Health(ctx context.Context) error
Close() error
NewReader(ctx context.Context, name string) (io.ReadCloser, error)
NewRangeReader(ctx context.Context, name string, offset, length int64) (io.ReadCloser, error)
NewWriter(ctx context.Context, name string) io.WriteCloser
DeleteObject(ctx context.Context, name string) error
CopyObject(ctx context.Context, src, dst string) error
StatObject(ctx context.Context, name string) (*ObjectInfo, error)
ListObjects(ctx context.Context, prefix string) ([]string, error)
ListDir(ctx context.Context, prefix string) ([]ObjectInfo, []string, error)
}Each backend implements this interface via its own storage_adapter.go, making the system modular, extensible, and easier to test. So updated GCS implementation includes:
Reason: StorageProvider is stateless (no auth, no file state), so we need:
I request all reviewers to have a look at the architecture and code after that i can complete the PR with updated tests and documentation. Please let me know if any changes in current implementation is required. |
Pull Request Template
Description:
pkg/datasource/file/gcs.Create,Remove,ReadDir,Open,StatandMakeDirusingcloud.google.com/go/storage.Checklist:
goimportandgolangci-lint.Fixes #2013