Skip to content

Initial implimentation of the s3 provider#75

Open
ian-noaa wants to merge 3 commits into
add-storage-provider-interfacefrom
add-s3-storage-provider
Open

Initial implimentation of the s3 provider#75
ian-noaa wants to merge 3 commits into
add-storage-provider-interfacefrom
add-s3-storage-provider

Conversation

@ian-noaa

Copy link
Copy Markdown
Collaborator

Add an s3 provider to unpack a gzipped tarball of stat files from an S3 bucket. Additionally adds tests & some documentation.

@ian-noaa ian-noaa self-assigned this Jun 24, 2026
@ian-noaa ian-noaa linked an issue Jun 25, 2026 that may be closed by this pull request
Add an s3 provider to unpack a gzipped tarball of stat files from
an S3 bucket. Additionally adds tests & some documentation.
@ian-noaa
ian-noaa force-pushed the add-s3-storage-provider branch from 178fdfe to 045d1f6 Compare June 25, 2026 22:02
ian-noaa added 2 commits June 25, 2026 16:04
Also add a note that I'm not sure we're safe in a concurrent situation
@ian-noaa
ian-noaa force-pushed the add-s3-storage-provider branch from cc9e510 to 340502e Compare June 30, 2026 18:18
@ian-noaa
ian-noaa marked this pull request as ready for review July 15, 2026 15:35
@gopa-noaa

Copy link
Copy Markdown
Contributor

Hi, Ian, I have some notes on setting up localstack on Mac/Linux. Since you got this working with Ministack, may be it is a good idea to capture these in some place.

@ian-noaa

Copy link
Copy Markdown
Collaborator Author

Thanks for checking this out Gopa! I did add a few notes on running ministack for the test suite in the dev guide. (linked below) Otherwise, all the ministack & test configuration is done via the test suite - fortunately, getting it going is a docker run and you're done. Is there more that would be helpful to add to the notes?

https://github.com/dtcenter/METjson2db/pull/75/changes#diff-fbdffcced43ae51406a84b80596bf79e8fdb08853cabe5cc17806f049791ba50R32-R55

@gopa-noaa gopa-noaa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an S3-backed storage provider and supporting utilities so the pipeline can stream a gzipped tarball from S3 and process only .stat / .tcst entries, alongside unit + integration test coverage and a developer guide for running tests locally.

Changes:

  • Introduces S3TarballProvider implementing storage.StorageProvider by streaming + iterating .stat/.tcst entries from an S3-hosted .tar.gz.
  • Adds ParseS3Event for decoding S3 event notification payloads (including URL-decoding object keys).
  • Adds unit tests and an integration test suite (MiniStack-based) plus developer documentation for running tests/coverage.

Reviewed changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
pkg/storage/s3tarball.go New S3 tarball storage provider that walks stat entries from an S3 GetObject stream
pkg/storage/s3tarball_test.go Unit tests for stat-file detection and tarball walking behavior (filtering, cancellation, errors)
pkg/storage/s3event.go New S3 event JSON parsing helper with key URL-decoding
pkg/storage/s3event_test.go Unit tests covering S3 event parsing scenarios
pkg/storage/s3_integration_test.go MiniStack integration test that uploads a tarball to S3 and validates streaming via the provider
pkg/storage/fakes3_test.go Test fake implementing GetObject for provider unit tests
go.mod Adds AWS SDK v2 dependencies required for S3 support and integration tests
go.sum Records module sums for new AWS SDK v2 dependencies
docs/dev-guide.md New developer guide documenting build/test/integration test setup (MiniStack + Couchbase)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pkg/storage/s3tarball.go
Comment on lines +82 to +85
fileCount++
if err := fn(hdr.Name, tr); err != nil { // Note - this most likely isn't safe in a concurrent environment given the global state
return err
}
Comment on lines +130 to +140
client.CreateBucket(ctx, &s3.CreateBucketInput{
Bucket: aws.String(testBucket),
})

_, err := client.PutObject(ctx, &s3.PutObjectInput{
Bucket: aws.String(testBucket),
Key: aws.String(testKey),
Body: bytes.NewReader(tarball),
ContentType: aws.String("application/gzip"),
ChecksumAlgorithm: "SHA256",
})
if resp.StatusCode != http.StatusOK {
t.Fatalf(
"MiniStack health check returned status %d at %s\n\n"+
"Ensure MiniStack is running: localstack start",
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update METjson2db to interact with object storage

3 participants