-
Notifications
You must be signed in to change notification settings - Fork 1
feat: add Go SDK #52
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
feat: add Go SDK #52
Conversation
Add comprehensive documentation for AI agents working on this project, including: - Conventional commits format and attribution requirements - Project structure overview for JavaScript/TypeScript and Go SDKs - Development workflows and testing guidelines - Release process information Assisted-by: GLM 4.7 via Claude Code
Rename .github/workflows/test.yaml to .github/workflows/javascript.yaml to better distinguish it from the Go workflow. Assisted-by: GLM 4.7 via Claude Code
Add comprehensive Go SDK for Tigris object storage including: - S3-compatible client implementation using AWS SDK v2 - Storage operations (Put, Get, Delete, List, etc.) - Tigris-specific headers support - CI workflow for Go testing - Module documentation in README Assisted-by: GLM 4.7 via Claude Code
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.
Greptile Overview
Greptile Summary
Adds a comprehensive Go SDK for Tigris storage with S3-compatible wrappers and Tigris-specific features like bucket forking, snapshots, and object renaming.
Major changes:
- New
storagepackage with client initialization and Tigris-specific operations tigrisheaderssubpackage for setting Tigris HTTP headers- Comprehensive test coverage and example code
- GitHub Actions workflow for cross-platform testing
- Complete documentation in README
Issues found:
- Credential handling bug that overrides AWS config defaults
- Unsafe type assertion that can panic
- Invalid GitHub Actions runner labels for ARM platforms
- Potential header parsing issues with special characters in snapshot descriptions
Confidence Score: 3/5
- PR has critical bugs that will cause runtime failures
- Three critical logic errors identified: credential handling that breaks AWS config, unsafe type assertion that can panic, and invalid CI runner configuration that will fail. These must be fixed before merge.
go/storage.go(credential bug),go/client.go(panic risk),.github/workflows/go.yaml(invalid runners)
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| go/storage.go | 3/5 | Main client creation with credential handling bug that overrides AWS config credentials |
| go/client.go | 3/5 | Tigris-specific methods with unsafe type assertion and missing snapshot version support for forks |
| .github/workflows/go.yaml | 2/5 | Go CI workflow with invalid ARM runner labels that will fail |
| go/tigrisheaders/tigrisheaders.go | 4/5 | Header helpers with potential parsing issues in snapshot descriptions |
Add proper error handling for the middleware response type assertion to prevent potential panics from unsafe type casts.
fix(go): URL-escape snapshot description in WithTakeSnapshot - Add godoc comment mentioning WithSnapshotVersion option - Fix snapshot description to be properly URL-encoded
Prevent setting nil credentials in AWS config, allowing the credential chain to run its default resolution process.
|
@greptileai can you re-review please? |
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.
Greptile Overview
Greptile Summary
Summary
This PR introduces a Go SDK for Tigris Storage with comprehensive wrapper functions and helper utilities. The implementation provides:
Key Components:
- Client Wrapper (
go/client.go): High-level methods wrapping AWS S3 SDK for Tigris-specific features including bucket snapshots, forks, and object renaming - Storage Configuration (
go/storage.go): Client initialization with functional options pattern for configuring endpoints, regions, and credentials - Header Utilities (
go/tigrisheaders/): Helper functions for setting Tigris-specific HTTP headers on S3 operations
Implementation Quality:
- Well-structured with clear separation of concerns
- Comprehensive test coverage including edge cases and benchmark tests
- Example functions demonstrating all major features
- Proper error handling with wrapped error messages
- Correct implementation of AWS SDK v2 patterns
- Follows Go conventions and best practices
Documentation:
- Detailed README with installation, configuration, and usage examples
- Updated AGENTS.md with Go SDK development guidelines
- Clear docstring comments on exported functions
- Example tests that serve as executable documentation
Testing:
- Comprehensive unit tests for options and client creation
- Extensive test coverage in
tigrisheaders_test.gowith edge cases - Benchmark tests for performance-critical operations
- Cross-platform CI testing (Linux, Windows, macOS, ARM architectures)
- Static analysis with
go vetand staticcheck
Issues Found:
Two documentation bugs were identified and fixed:
- Package comment in
storage.goincorrectly identifies the package as 'tigrissdk' instead of 'storage' - Comment for
WithCreateObjectIfNotExistsintigrisheaders.gowas missing the 'Object' prefix
Overall, the SDK provides a solid, well-tested foundation for Go developers to interact with Tigris Storage service.
Confidence Score: 4/5
- This PR is safe to merge with minor documentation corrections needed. The implementation is solid with comprehensive testing.
- The score reflects thorough implementation and extensive testing across multiple platforms. Two minor documentation bugs were found (incorrect package comment and function comment mismatch) that should be fixed before merge. The core logic is sound, credentials handling is correct, and the API design follows the specified custom instructions. All AWS SDK integration patterns are properly implemented. The score is not higher due to the documentation issues that need correction.
- Files with documentation issues: go/storage.go (package comment) and go/tigrisheaders/tigrisheaders.go (function comment). These should be corrected before merge but represent minor, non-functional issues.
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| go/storage.go | 4/5 | Package comment incorrectly identifies the package as 'tigrissdk' when the actual package name is 'storage'. Main client initialization and configuration logic is sound. |
| go/client.go | 5/5 | Client wrapper implementation around AWS S3 with Tigris-specific methods. Methods properly delegate to underlying S3 client and add appropriate headers. |
| go/tigrisheaders/tigrisheaders.go | 4/5 | HTTP header helper functions for Tigris-specific features. Contains documentation error: comment on line 68 says 'WithCreateIfNotExists' but function is named 'WithCreateObjectIfNotExists'. |
| go/storage_test.go | 5/5 | Comprehensive test coverage for options, client creation, and functional option composition. Tests are well-structured and cover edge cases. |
| go/tigrisheaders/tigrisheaders_test.go | 5/5 | Extensive test coverage for header functions including edge cases, regional options, and benchmark tests. Tests verify proper APIOptions application. |
… in case Signed-off-by: Xe Iaso <xe@tigrisdata.com>
- Correct package description from "tigrissdk" to "storage" - Fix function name comment to match WithCreateObjectIfNotExists
Signed-off-by: Xe Iaso <xe@tigrisdata.com>
Signed-off-by: Xe Iaso <xe@tigrisdata.com>
|
@greptileai re-review please |
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.
Pull request overview
This pull request introduces a Go SDK for Tigris object storage, providing S3-compatible wrappers with Tigris-specific features. The SDK is built on top of the AWS SDK v2 for Go and includes functionality for bucket snapshots, forks, object renaming, and metadata querying.
Key Changes:
- Added Go SDK with two main packages:
storage(high-level client) andtigrisheaders(HTTP header helpers) - Implemented Tigris-specific features including bucket forking, snapshots, and in-place object renaming
- Comprehensive test coverage with unit tests, benchmarks, and example tests
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| go/storage.go | Main client package with configuration options and initialization logic |
| go/client.go | Client wrapper with Tigris-specific methods for snapshots, forks, and renaming |
| go/tigrisheaders/tigrisheaders.go | Header helper functions for Tigris-specific HTTP headers |
| go/tigrisheaders/tigrisheaders_test.go | Comprehensive unit tests for header functions |
| go/storage_test.go | Unit tests for client configuration and initialization |
| go/example_test.go | Example code demonstrating SDK usage |
| go/tigrisheaders/example_test.go | Example code for header helpers |
| go/README.md | Comprehensive documentation for the Go SDK |
| go.mod | Go module definition with dependencies |
| go.sum | Dependency checksums |
| .github/workflows/go.yaml | CI workflow for Go tests across multiple platforms |
| .github/workflows/javascript.yaml | Updated JavaScript test workflow name |
| AGENTS.md | Project documentation including Go SDK details |
| CLAUDE.md | Reference to AGENTS.md |
| README.md | Updated to include Go SDK reference |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Greptile Overview
Greptile Summary
This PR adds a well-structured Go SDK for Tigris Storage with S3-compatible methods and Tigris-specific features like bucket forking, snapshots, and object renaming.
Key additions:
- Clean client wrapper around AWS SDK v2 with functional options pattern
- Comprehensive header helpers in
tigrisheaderspackage - Excellent test coverage including unit tests, examples, and benchmarks
- Multi-platform CI workflow (Ubuntu, Windows, macOS on x86 and ARM)
- Thorough documentation with examples
Issue found:
- Snapshot header format uses
desc=but JavaScript SDK usesname=, causing potential API inconsistency
Confidence Score: 4/5
- Safe to merge after fixing the snapshot header format inconsistency
- The PR demonstrates excellent code quality with comprehensive testing and documentation. The only issue is a header format inconsistency between Go and JavaScript SDKs that could cause API compatibility problems.
- go/tigrisheaders/tigrisheaders.go - snapshot header format needs alignment with JS SDK
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| go/tigrisheaders/tigrisheaders.go | 4/5 | comprehensive header helpers with one inconsistency in snapshot format |
| go/client.go | 5/5 | well-structured client wrapper with clear Tigris-specific methods |
| go/storage.go | 5/5 | clean initialization logic with functional options pattern |
| go/README.md | 5/5 | comprehensive documentation with clear examples and feature coverage |
| .github/workflows/go.yaml | 5/5 | well-configured CI workflow with multi-platform testing |
go/tigrisheaders/tigrisheaders.go
Outdated
| // | ||
| // [1]: https://www.tigrisdata.com/docs/buckets/snapshots-and-forks/#creating-a-snapshot | ||
| func WithTakeSnapshot(desc string) func(*s3.Options) { | ||
| return WithHeader("X-Tigris-Snapshot", fmt.Sprintf("true; desc=%s", url.QueryEscape(desc))) |
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.
snapshot header format inconsistent with JS SDK - uses desc= but JS SDK uses name= (see packages/storage/src/lib/bucket/snapshot.ts line 140)
Prompt To Fix With AI
This is a comment left during a code review.
Path: go/tigrisheaders/tigrisheaders.go
Line: 130:130
Comment:
snapshot header format inconsistent with JS SDK - uses `desc=` but JS SDK uses `name=` (see `packages/storage/src/lib/bucket/snapshot.ts` line 140)
How can I resolve this? If you propose a fix, please make it concise.Change 'desc=' to 'name=' in WithTakeSnapshot to match the Tigris API specification for snapshot creation. Assisted-by: GLM 4.7 via Claude Code
|
🎉 This PR is included in version 1.0.10 🎉 The release is available on npm package (@latest dist-tag) Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.1.4 🎉 The release is available on npm package (@latest dist-tag) Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 2.11.0 🎉 The release is available on npm package (@latest dist-tag) Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.1.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Starting out with some simple wrappers around the S3 client structure. Will make a high level client later.