Skip to content

Conversation

@andrewmbenton
Copy link
Collaborator

Enable remote execution for sqlc generate when the "cloud" configuration block exists. To disable remote execution without altering your config, a new --no-remote flag is available.

While developing this I ran the entire endtoend test suite against the remote codegen service to confirm correctness. In this patch I include a single remote-specific test that covers both a successful code generation and an expected error. This test passes on my machine but is disabled by default, since it will always fail unless a locally-running implementation of the remote execution service is available.

Comment on lines +49 to +54

remote-proto:
protoc \
--go_out=. --go_opt="Minternal/remote/gen.proto=github.com/kyleconroy/sqlc/internal/remote" --go_opt=module=github.com/kyleconroy/sqlc \
--go-grpc_out=. --go-grpc_opt="Minternal/remote/gen.proto=github.com/kyleconroy/sqlc/internal/remote" --go-grpc_opt=module=github.com/kyleconroy/sqlc \
internal/remote/gen.proto
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should get this working using buf generate to ensure that we're consistently using the same code for gRPC. This isn't blocking.

}

if conf.Cloud.Hostname != "" && !e.NoRemote {
rpcClient, err := remote.NewClient(conf.Cloud)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of inlining this inside Generate, could we move it to a separate remoteGenerate function?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's a good idea. The function signature is a bit long but I think it's better. See changes incoming.

grpc.WithPerRPCCredentials(bearer.NewPerRPCCredentials(os.Getenv("SQLC_AUTH_TOKEN"))),
}

conn, err := grpc.Dial(cloudConfig.Hostname+":443", opts...)
Copy link
Collaborator

Choose a reason for hiding this comment

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

The hostname currently doesn't default to anything other than an empty string. Should we pick a default now?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The empty string default ends up as the equivalent of "localhost" in the current implementation. We could add an explicit default though. To "localhost", or something else?

@kyleconroy kyleconroy merged commit 3872990 into sqlc-dev:main Apr 19, 2023
alfonsodev pushed a commit to ExponentiaTeam/sqlc that referenced this pull request Oct 13, 2025
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.

2 participants