spanner: admin client does not respect SPANNER_EMULATOR_HOST

[kazegusuri]

When we set an address to SPANNER_EMULATOR_HOST environment variable, spanner.NewSpannerClient() respects the value in SDK to connect an emulator. But admin clients for instance and database does not respect. It is useful to operate databases in tests or tools.

[jeanbza]

To be clear - are you describing https://godoc.org/cloud.google.com/go/spanner/admin/database/apiv1 and https://godoc.org/cloud.google.com/go/spanner/admin/instance/apiv1?

[kazegusuri]

yes, right.

[jeanbza]

Gotcha. Thanks. We can't change those auto-generated clients, but there might be a way to get what you need through the https://godoc.org/google.golang.org/api/option passed to the NewClient call. @olavloite - do you have some free cycles to investigate?

[olavloite]

You can setup a connection to the emulator for the DatabaseAdminClient like this:

    ctx := context.Background()
    // Setup an emulator server.
    srv, err := spannertest.NewServer("localhost:0")
    if err != nil {
        t.Fatalf("Starting in-memory fake: %v", err)
    }
    srv.SetLogger(t.Logf)
    dialCtx, cancel := context.WithTimeout(ctx, 1*time.Second)
    defer cancel()
    // Setup connection to the emulator server. Note that we use an insecure
    // connection as the emulator does not use https.
    conn, err := grpc.DialContext(dialCtx, srv.Addr, grpc.WithInsecure())
    if err != nil {
        srv.Close()
        t.Fatalf("Dialing in-memory fake: %v", err)
    }
    // Create an admin client that connects to the emulator.
    adminClient, err := dbadmin.NewDatabaseAdminClient(ctx, option.WithGRPCConn(conn))
    if err != nil {
        srv.Close()
        t.Fatalf("Connecting to in-memory fake DB admin: %v", err)
    }
    // Create a table on the emulator to verify the connection.
    op, err := adminClient.UpdateDatabaseDdl(ctx, &database.UpdateDatabaseDdlRequest{
        Statements: []string {"CREATE TABLE FOO (ID INT64, NAME STRING(100)) PRIMARY KEY (ID)"},
    })
    if err != nil {
        t.Fatalf("Create table failed: %v", err)
    }
    err = op.Wait(ctx)
    if err != nil {
        t.Fatalf("Create table failed: %v", err)
    }
    t.Log("Table created")
    adminClient.Close()
    conn.Close()
    srv.Close()

I'll add this to the examples file as well.

[kazegusuri]

This is not a question actually. I know how to specify an address in admin client as you mentioned above. I hope google-cloud-go library supports to use an address in SPANNER_EMULATER_HOST for admin client implicitly as spanner.NewClient() does.

[olavloite]

Sorry, I misunderstood that.

As Jean mentioned above, we cannot change the auto-generated admin clients. One possibility could be to add a new helper function NewDatabaseAdminClient(...) that creates a DatabaseAdminClient pointing either to the production server or the emulator, depending on the environment variable. It would however still mean that you would need to change your existing code to use the new helper method instead of directly creating a DatabaseAdminClient.

[kazegusuri]

Thank you for the reply.

Yes, even if we need to change to use a new function, it's very appreciate. Now we use code something like above to change the address to emulator explicitly for admin clients when the environment variable is set. If admin clients also behave like a normal spanner client, we can transparently switch the backend by the variable.

[jeanbza]

If admin clients also behave like a normal spanner client, we can transparently switch the backend by the variable.

You can still achieve this by wrapping the code @olavloite in your own function. There's no difference between his block of code and a new helper except for quality of life improvements.

[kazegusuri]

Yes we can do in application side. That can be said for the normal licnet as well. If we can switcht a backend by setting env for the normal client, we expect that for other clients. It's confusing if only a part of them has different behavior.

[jeanbza]

That's reasonable. @noahdietz WDYT about extending the microgenerator to support this kind of thing? It seems spanner-specific at first glance but I think is actually broadly applicable - any gapic could in theory have an emulator backing it.

[noahdietz]

Helllo! An unofficial threshold we have is that there must be at least 5 other APIs that want such a feature to add it to a generator. It has been said in the past that "any gapic could in theory" use feature X, but then in practice only the one API requesting the feature actually uses it. Then this becomes a one-off. Do we have 5 such APIs that use emulators or planned work in the near future to expand emulator use?

Implementation-wise, I think this would be better suited by a new client construction helper in the GAPIC as suggested in the thread, not by reading from an emulator-specific environment variable. Other GAPIC languages have multiple client "constructors" that could take a gRPC Channel, an address string or that just use the default host automatically. Go only has the third type in that list requiring the use of all the options/setup as @olavloite demonstrated (which is fine, just makes it more laborious to use for the situation in question).

Open to exploring this, but I would like to see help it address the broader problem of targeting not-Google with a GAPIC.

[jeanbza]

@noahdietz We have 4 today: spanner, pubsub, firestore, bigtable, datastore. This would add a fifth, so we just scrape by. :)

Implementation-wise, I think this would be better suited by a new client construction helper in the GAPIC as suggested in the thread, not by reading from an emulator-specific environment variable.

Ack. Unfortunately, that doesn't work across SDKs - for example, gcloud respects the environment variable. I think there are other historical reasons why former teams went this direction, but that's the one that comes to mind.

[noahdietz]

This would add a fifth, so we just scrape by. :)

Ok, we are in business!

that doesn't work across SDKs - for example, gcloud respects the environment variable

Ok I would like to sync up with gcloud & other generator languages. Just because it's the current way doesn't mean it's the right/scalable/broadly applicable way :) But also because I don't want to do something different than everyone else by accident. So I kindly ask for some time to do so.

[jeanbza]

Perfect - thanks Noah. Could we close this issue and create a tracking FR in the appropriate generator repository?

[noahdietz]

Closing in favor of https://github.com/googleapis/gapic-generator-go/issues/230

[vvakame]

@noahdietz this pull request was closed that https://github.com/googleapis/gapic-generator-go/issues/230 is opened. but that issue was closed. so, I think this issue should be reopen ✨

[noahdietz]

@vvakame I opened #1617 as a specific task to include documentation examples on how to dial an emulator. Per the explanation in https://github.com/googleapis/gapic-generator-go/issues/230, the generated code will not be changed to support this. Thanks.