feat(bridge-history): add aws s3 blob client #1716
Conversation
WalkthroughAdded Changes
Sequence Diagram(s)sequenceDiagram
participant Config
participant L1Fetcher
participant BlobClients
participant AwsS3
Config->>L1Fetcher: Provide FetcherConfig (includes AwsS3Endpoint)
L1Fetcher->>BlobClients: Initialize base blob clients
alt AwsS3Endpoint != ""
L1Fetcher->>AwsS3: Create AWS S3 blob client
AwsS3-->>BlobClients: Add AWS S3 client to collection
end
L1Fetcher-->>BlobClients: Add other conditional blob clients (BeaconNode, BlobScan, BlockNative)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
common/version/version.go (1)
8-8: Incremented tag looks fine – remember to update release artifacts/changelog.The version bump itself is correct; just ensure accompanying release notes and deployment manifests get the same tag so CI/CD picks it up.
bridge-history-api/internal/config/config.go (1)
41-42: Field name casing is inconsistent with existing acronym rules.Previous acronym-based fields keep the acronym fully-capitalised (
ETHGatewayAddr,BeaconNodeAPIEndpoint). For consistency the new field should probably beAWSS3Endpoint(JSON tag"AWSS3Endpoint"). Keeping it mixed-case (Aws) will work technically, but introduces naming drift in code, JSON, environment-variable overrides and documentation.If you decide to align casing, the diff would be:
- AwsS3Endpoint string `json:"AwsS3Endpoint"` + AWSS3Endpoint string `json:"AWSS3Endpoint"`
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
bridge-history-api/internal/config/config.go(1 hunks)bridge-history-api/internal/controller/fetcher/l1_fetcher.go(1 hunks)common/version/version.go(1 hunks)
🔇 Additional comments (1)
bridge-history-api/internal/controller/fetcher/l1_fetcher.go (1)
42-44: Verify AWS S3 client instantiation and configuration
In
bridge-history-api/internal/controller/fetcher/l1_fetcher.go(lines 42–44), wrap the S3 client constructor in error handling, matching the Beacon-Node pattern:- if cfg.AwsS3Endpoint != "" { - blobClient.AddBlobClient(blob_client.NewAwsS3Client(cfg.AwsS3Endpoint)) - } + if cfg.AwsS3Endpoint != "" { + s3Client, err := blob_client.NewAwsS3Client(cfg.AwsS3Endpoint) + if err != nil { + log.Warn("failed to create AwsS3Client", "err", err) + } else { + blobClient.AddBlobClient(s3Client) + } + }Confirm that
blob_client.NewAwsS3Clientreturns(Client, error)and adjust the call site accordingly.Ensure
AwsS3EndpointinFetcherConfigsuffices for S3 usage—if not, extend the config with fields for region, bucket name, or credential sources (or document reliance on environment/IAM roles).
3 participants
Purpose or design rationale of this PR
Describe your change. Make sure to answer these three questions: What does this PR do? Why does it do it? How does it do it?
PR title
Your PR title must follow conventional commits (as we are doing squash merge for each PR), so it must start with one of the following types:
Deployment tag versioning
Has
tagincommon/version.gobeen updated or have you addedbump-versionlabel to this PR?Breaking change label
Does this PR have the
breaking-changelabel?Summary by CodeRabbit
New Features
Chores