-
-
Notifications
You must be signed in to change notification settings - Fork 502
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 support to seed data when using RunBigQueryContainer #2523
base: main
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,7 +13,7 @@ import ( | |
func RunBigQueryContainer(ctx context.Context, opts ...testcontainers.ContainerCustomizer) (*GCloudContainer, error) { | ||
req := testcontainers.GenericContainerRequest{ | ||
ContainerRequest: testcontainers.ContainerRequest{ | ||
Image: "ghcr.io/goccy/bigquery-emulator:0.4.3", | ||
Image: "ghcr.io/goccy/bigquery-emulator:0.6.1", | ||
ExposedPorts: []string{"9050/tcp", "9060/tcp"}, | ||
WaitingFor: wait.ForHTTP("/discovery/v1/apis/bigquery/v2/rest").WithPort("9050/tcp").WithStartupTimeout(time.Second * 5), | ||
}, | ||
|
@@ -27,18 +27,24 @@ func RunBigQueryContainer(ctx context.Context, opts ...testcontainers.ContainerC | |
|
||
req.Cmd = []string{"--project", settings.ProjectID} | ||
|
||
for _, opt := range opts { | ||
if err := opt.Customize(&req); err != nil { | ||
return nil, err | ||
} | ||
} | ||
|
||
container, err := testcontainers.GenericContainer(ctx, req) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
spannerContainer, err := newGCloudContainer(ctx, 9050, container, settings) | ||
bigqueryContainer, err := newGCloudContainer(ctx, 9050, container, settings) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like this needs a rebase as there has been some work to ensure this correctly returns the container even on error. |
||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
// always prepend http:// to the URI | ||
spannerContainer.URI = "http://" + spannerContainer.URI | ||
bigqueryContainer.URI = "http://" + bigqueryContainer.URI | ||
|
||
return spannerContainer, nil | ||
return bigqueryContainer, nil | ||
mtellis2 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} |
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -5,6 +5,8 @@ import ( | |||||||||
"errors" | ||||||||||
"fmt" | ||||||||||
"log" | ||||||||||
"path/filepath" | ||||||||||
"testing" | ||||||||||
|
||||||||||
"cloud.google.com/go/bigquery" | ||||||||||
"google.golang.org/api/iterator" | ||||||||||
|
@@ -13,17 +15,19 @@ import ( | |||||||||
"google.golang.org/grpc" | ||||||||||
"google.golang.org/grpc/credentials/insecure" | ||||||||||
|
||||||||||
"github.com/stretchr/testify/assert" | ||||||||||
"github.com/stretchr/testify/require" | ||||||||||
"github.com/testcontainers/testcontainers-go" | ||||||||||
"github.com/testcontainers/testcontainers-go/modules/gcloud" | ||||||||||
) | ||||||||||
|
||||||||||
func ExampleRunBigQueryContainer() { | ||||||||||
func TestBigQueryContainer(t *testing.T) { | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We would like to keep some testable examples, as they will land in the pkg.go.dev docs. If you have doubts on when creating a real test or a testable example, please use a testable example when you want to show case how to use the module. Else, a regular test is fine. |
||||||||||
// runBigQueryContainer { | ||||||||||
ctx := context.Background() | ||||||||||
|
||||||||||
bigQueryContainer, err := gcloud.RunBigQueryContainer( | ||||||||||
ctx, | ||||||||||
testcontainers.WithImage("ghcr.io/goccy/bigquery-emulator:0.4.3"), | ||||||||||
testcontainers.WithImage("ghcr.io/goccy/bigquery-emulator:0.6.1"), | ||||||||||
gcloud.WithProjectID("bigquery-project"), | ||||||||||
) | ||||||||||
if err != nil { | ||||||||||
|
@@ -78,8 +82,85 @@ func ExampleRunBigQueryContainer() { | |||||||||
} | ||||||||||
} | ||||||||||
|
||||||||||
fmt.Println(val) | ||||||||||
// Output: | ||||||||||
// [30] | ||||||||||
expectedValue := int64(30) | ||||||||||
actualValue := val[0] | ||||||||||
fmt.Println(val[0]) | ||||||||||
|
||||||||||
require.NoError(t, err) | ||||||||||
if assert.NotNil(t, val) { | ||||||||||
assert.Equal(t, expectedValue, actualValue) | ||||||||||
} | ||||||||||
mtellis2 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
} | ||||||||||
|
||||||||||
func TestBigQueryWithDataYamlFile(t *testing.T) { | ||||||||||
// runBigQueryContainer { | ||||||||||
mtellis2 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
ctx := context.Background() | ||||||||||
|
||||||||||
absPath, err := filepath.Abs(filepath.Join(".", "testdata", "data.yaml")) | ||||||||||
if err != nil { | ||||||||||
log.Fatalf("failed to run container: %v", err) | ||||||||||
} | ||||||||||
Comment on lines
+92
to
+94
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: use require.NoError to simplify, more below
Suggested change
|
||||||||||
|
||||||||||
bigQueryContainer, err := gcloud.RunBigQueryContainer( | ||||||||||
ctx, | ||||||||||
testcontainers.WithImage("ghcr.io/goccy/bigquery-emulator:0.6.1"), | ||||||||||
gcloud.WithProjectID("test"), | ||||||||||
gcloud.WithDataYamlFile(absPath), | ||||||||||
) | ||||||||||
if err != nil { | ||||||||||
log.Fatalf("failed to run container: %v", err) | ||||||||||
} | ||||||||||
|
||||||||||
// Clean up the container | ||||||||||
defer func() { | ||||||||||
if err := bigQueryContainer.Terminate(ctx); err != nil { | ||||||||||
log.Fatalf("failed to terminate container: %v", err) | ||||||||||
} | ||||||||||
}() | ||||||||||
Comment on lines
+106
to
+110
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: use |
||||||||||
// } | ||||||||||
|
||||||||||
// bigQueryClient { | ||||||||||
projectID := bigQueryContainer.Settings.ProjectID | ||||||||||
|
||||||||||
opts := []option.ClientOption{ | ||||||||||
option.WithEndpoint(bigQueryContainer.URI), | ||||||||||
option.WithGRPCDialOption(grpc.WithTransportCredentials(insecure.NewCredentials())), | ||||||||||
option.WithoutAuthentication(), | ||||||||||
internaloption.SkipDialSettingsValidation(), | ||||||||||
} | ||||||||||
|
||||||||||
client, err := bigquery.NewClient(ctx, projectID, opts...) | ||||||||||
if err != nil { | ||||||||||
log.Fatalf("failed to create bigquery client: %v", err) // nolint:gocritic | ||||||||||
} | ||||||||||
defer client.Close() | ||||||||||
// } | ||||||||||
|
||||||||||
selectQuery := client.Query("SELECT * FROM dataset1.table_a where name = @name") | ||||||||||
selectQuery.QueryConfig.Parameters = []bigquery.QueryParameter{ | ||||||||||
{Name: "name", Value: "bob"}, | ||||||||||
} | ||||||||||
it, err := selectQuery.Read(ctx) | ||||||||||
if err != nil { | ||||||||||
log.Fatalf("failed to read query: %v", err) | ||||||||||
} | ||||||||||
|
||||||||||
var val []bigquery.Value | ||||||||||
for { | ||||||||||
err := it.Next(&val) | ||||||||||
if errors.Is(err, iterator.Done) { | ||||||||||
break | ||||||||||
} | ||||||||||
if err != nil { | ||||||||||
log.Fatalf("failed to iterate: %v", err) | ||||||||||
} | ||||||||||
} | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. bug: left over output comments below. |
||||||||||
|
||||||||||
// Output: | ||||||||||
// [30] | ||||||||||
expectedValue := int64(30) | ||||||||||
actualValue := val[0] | ||||||||||
assert.Equal(t, expectedValue, actualValue) | ||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -41,12 +41,14 @@ func newGCloudContainer(ctx context.Context, port int, c testcontainers.Containe | |
} | ||
|
||
type options struct { | ||
ProjectID string | ||
ProjectID string | ||
DataYamlFile string | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. bug: this doesn't seem to be ever used. |
||
} | ||
|
||
func defaultOptions() options { | ||
return options{ | ||
ProjectID: defaultProjectID, | ||
ProjectID: defaultProjectID, | ||
DataYamlFile: "/data.yaml", | ||
} | ||
} | ||
|
||
|
@@ -69,6 +71,22 @@ func WithProjectID(projectID string) Option { | |
} | ||
} | ||
|
||
// WithDataYamlFile seeds the Bigquery project for the GCloud container. | ||
func WithDataYamlFile(dataYamlFile string) testcontainers.CustomizeRequestOption { | ||
Comment on lines
+74
to
+75
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: remove the duplication between function name and paramater |
||
return func(req *testcontainers.GenericContainerRequest) error { | ||
mtellis2 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
dataFile := testcontainers.ContainerFile{ | ||
HostFilePath: dataYamlFile, | ||
ContainerFilePath: "/data.yaml", | ||
FileMode: 0o755, | ||
} | ||
|
||
req.Files = append(req.Files, dataFile) | ||
req.Cmd = append(req.Cmd, "--data-from-yaml", "/data.yaml") | ||
|
||
return nil | ||
} | ||
} | ||
|
||
// applyOptions applies the options to the container request and returns the settings. | ||
func applyOptions(req *testcontainers.GenericContainerRequest, opts []testcontainers.ContainerCustomizer) (options, error) { | ||
settings := defaultOptions() | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
projects: | ||
- id: test | ||
datasets: | ||
- id: dataset1 | ||
tables: | ||
- id: table_a | ||
columns: | ||
- name: id | ||
type: INTEGER | ||
- name: name | ||
type: STRING | ||
- name: createdAt | ||
type: TIMESTAMP | ||
data: | ||
- id: 1 | ||
name: alice | ||
createdAt: "2022-10-21T00:00:00" | ||
- id: 30 | ||
name: bob | ||
createdAt: "2022-10-21T00:00:00" |
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.
Mmm I think the applyOptions call will already do this. Please see https://github.com/testcontainers/testcontainers-go/pull/2523/files#diff-658ed21fe179e3c4810748c731c20b75757c269dd8ce30f0bf98ce31f2addd24R91-R103
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.
@mdelapenya it wasn't working before without this and that appears to be due to needing to update
req.Cmd = []string{"--project", settings.ProjectID}
toreq.Cmd = append(req.Cmd, "--project", settings.ProjectID)
so that the options added to the cmd are not removedThere 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.
Let me test this locally, from this branch. Will come back to you later
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.
Indeed, with the addition of the YAML data file, now it's possible to have different settings contributing to the CMD. So it's fine if we append.
My only concern is now related to the usage of the gcloud options across all the emulators 🤔 Do you know if all of them would support passing this YAML file? Else, anybody could add this functional option to e.g. Firestore or Pubsub, and expect something else. I'm not an expert in those services, so you probably have more insights.
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.
Good point, I myself have not used the other emulators but would assume this data yaml functionality would and should only apply for the bigquery emulator. I'm currently not sure of a great way to prevent this in the other gcloud emulators. I think the data.yaml option should be ignored though due to how that CMD is being set.
req.Cmd = []string{...}
It looks like the spanner container might be the only one that differs. It does not have
req.Cmd = []string{...}