feat: support injected verb clients in Go #2828
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
worstell
requested review from
a team and
gak
and removed request for
a team
worstell
force-pushed
the
worstell/20240925-inject-verbs-go-trimmed
branch
2 times, most recently
from
5fe4861
to
a48563b
Compare
alecthomas
approved these changes
Sep 26, 2024
| @@ -24,8 +24,8 @@ type EchoResponse struct { | |||
| // Echo returns a greeting with the current time. | |||
| // | |||
| //ftl:verb | |||
| func Echo(ctx context.Context, req EchoRequest) (EchoResponse, error) { | |||
| tresp, err := ftl.Call(ctx, time.Time, time.TimeRequest{}) | |||
| func Echo(ctx context.Context, req EchoRequest, tc time.TimeClient) (EchoResponse, error) { | |||
| @@ -18,7 +18,7 @@ type EchoResponse struct { | |||
| } | |||
|
|
|||
| //ftl:verb | |||
| func Echo(ctx context.Context, req EchoRequest) (EchoResponse, error) { | |||
| ftl.Call(ctx, other.Echo, other.EchoRequest{}) | |||
| func Echo(ctx context.Context, req EchoRequest, oc other.EchoClient) (EchoResponse, error) { | |||
There was a problem hiding this comment.
How do we know which parameter is the request? Must it always be the 2nd?
There was a problem hiding this comment.
in this implementation yes, though maybe it could be inferred? the first non-resource-typed param (where verb clients are functions and other resources are known types, e.g. ftl.PostgresDatabase etc.) we encounter
maybe worth landing the other resource types first and then revisiting this
There was a problem hiding this comment.
current approach allows a request as the 2nd parameter, optionally. if it's not there we just start consuming resources
worstell
force-pushed
the
worstell/20240925-inject-verbs-go-trimmed
branch
6 times, most recently
from
c7c5864
to
06a20eb
Compare
worstell
force-pushed
the
worstell/20240925-inject-verbs-go-trimmed
branch
from
775cc98
to
54b239d
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
https://hackmd.io/OULeRFQQQvaMURysN27eEw
only the TestLifecycle integration test is updated with the new approach in this PR, to demonstrate that both old and new verb call strategies are still functional. will remove other usages of
ftl.Call(...)to follow<Verb>Clientsignatures in external stubs and (local) signatures intypes.ftl.gomain.goandtypes.ftl.go, e.g. "providers" for verb clients-
go-runtime/build.gochanges accompanying the above^ bunch of stuff to get imports working now that we're generating full verb request/response types rather than just function referencesgo-runtime/server.goto inject the registered providers when processing an inbound call#2641