Add support for input plane invocation by rculbertson · Pull Request #32 · modal-labs/libmodal

rculbertson · 2025-06-17T01:24:29Z

~~This is based on #31.~~

This PR adds a new InputPlaneInvocation class, which sends inputs to the input plane, rather than the control plane. This will be used only for function definitions which include the input_plane_region experimental option like this:

@app.function(experimental_options={"input_plane_region": "us-east"})

If not specified, the ControlPlaneInvocation will be used.

~~For now this includes only the typescript implementation - if this approach looks good, we will add the go implementation.~~

rculbertson · 2025-06-20T15:45:36Z

The PR this was based on has been merged to main. This PR has been rebased on main, and is ready for review.

ekzhang · 2025-06-26T14:51:13Z

modal-js/src/client.ts

 import { ClientType, ModalClientDefinition } from "../proto/modal_proto/api";
 import { type Profile, profile } from "./config";

+let modalAuthToken: string | undefined;


This is a global variable right now and will end up shared between multiple clients. Instead I think it needs to be declared in the authMiddleware() factory between line 19 function authMiddleware(...) and line 20 return async function* authMiddleware(...), so it doesn't end up becoming accidentally shared global state.

We actually intentionally share the token between the control plane client and the input plane client. Here's the flow:

We call the control plane, and do not specify an auth token (because we don't have one yet)

The control plane sees there is no auth token - so it generates one, and returns it to the client

The client the specifies that auth token in every subsequent request to both the control plane and the input plane.
I will add a comment explaining this.

ekzhang · 2025-06-26T14:51:52Z

modal-js/src/client.ts

+      if (prevOnHeader) {
+        prevOnHeader(header);
+      }


prevOnHeader?.(header);

ekzhang · 2025-06-26T14:54:16Z

modal-js/src/invocation.ts

+    } catch (err) {
+      throw new Error(`FunctionGetOutputs failed: ${err}`);
+    }


I don't think we should catch this error this way since it erases the error details and status code from the structured Status response, and turns it into a string. Also the grpc error already has the method name in the stack, so this isn't necessary.

My thought process on errors is, in theory we would have every single possible turn into a human-readable format that is intelligible to users. In practice there are a lot of unanticipated, and the set of actual errors that users experience in practice is much smaller and more tractable to manage — so we let errors propagate by default, and we catch errors strategically based on which ones are actually expected to happen from library use (NotFoundError when you get the name of a function wrong, OutputExpired when you poll too late, etc.) and make them public API instead of an opaque Error type.

Sounds reasonable to me! Will remove the catch.

ekzhang · 2025-06-26T15:00:32Z

test-support/libmodal_test_support.py


+@app.function(min_containers=1, experimental_options={"input_plane_region": "us-west"})
+def input_plane(s: str) -> str:
+    return "output: " + s


nit: can you keep formatting with black?

ekzhang · 2025-06-26T15:05:32Z

modal-js/src/invocation.ts

+/**
+ * Signature of a function that fetches a single output. Used by `pollForOutputs` to fetch from either
+ * the control plane or the input plane, depending on the implementation.
+ */
+type GetOutput = (
+  timeoutMillis: number,
+) => Promise<FunctionGetOutputsItem | undefined>;


I think if we make a limited utility type like this that's used in one place, we should define it next to where it's used in pollFunctionOutput().

The documentation/naming on this method and type pollFunctionOutput() and GetOutput could be improved. It's not clear that what it's actually doing right now is, wrapping an output fetcher that's limited to the outputsTimeout constant (55 s) with a loop and continued polling so that it takes any timeout.

Also the naming is different between Go and JS of the interface, the identifiers called awaitOutput() / GetOutput / pollFunctionOutput() in JS correspond to getOutput() / getOutput / pollFunctionOutput() in Go, which is a bit confusing too because of the double-naming.

ekzhang · 2025-06-26T15:10:14Z

modal-go/function.go

+	// Attach x-modal-auth-token to all future requests.
+	authTokenArray := header.Get("x-modal-auth-token")
+	if len(authTokenArray) == 0 {
+		authTokenArray = trailer.Get("x-modal-auth-token")
+	}
+	if len(authTokenArray) > 0 {
+		authToken := authTokenArray[0]
+		ctx = metadata.AppendToOutgoingContext(ctx, "x-modal-auth-token", authToken)
+	}


Also isn't this middleware implementation … different from how x-modal-auth-token is being handled in modal-js? One of them will save auth tokens per client/globally, and one will save per function. I think it's really important that we try to make the client libraries behave the same way.

Sounds good. I will update the go code to match the typescript.

rculbertson · 2025-06-30T22:20:58Z

I addressed the PR comments, so this is ready for another review!

ehdr

This looks good to me!

ehdr · 2025-07-01T13:18:34Z

modal-js/src/invocation.ts

+      });
+      return response.output;
+    } catch (err) {
+      throw new Error(`AttemptAwait failed: ${err}`);


This should maybe also not be caught, with similar reasoning as #32 (comment)?

Ah, yes! I pushed another commit removing that.

rculbertson · 2025-07-02T14:14:23Z

Thank you @ekzhang and @ehdr for reviewing!

ekzhang · 2025-07-02T14:15:46Z

Yes, and sorry for any delays!

* Add support for input plane * Address PR review comments * Remove catch

rculbertson mentioned this pull request Jun 17, 2025

Add InvocationStrategy for sending inputs to control plane #31

Merged

rculbertson force-pushed the ryan/control-plane-strategy branch from 291fad9 to de7c480 Compare June 17, 2025 01:40

rculbertson force-pushed the ryan/input-plane-strategy branch 4 times, most recently from 57703ea to 0ce79ee Compare June 17, 2025 19:37

rculbertson force-pushed the ryan/control-plane-strategy branch from cacbb31 to 23a98f7 Compare June 18, 2025 15:57

rculbertson force-pushed the ryan/input-plane-strategy branch from 0ce79ee to f65d93e Compare June 18, 2025 18:23

rculbertson changed the title ~~Add input plane strategy~~ Add support for input plane invocation Jun 18, 2025

rculbertson force-pushed the ryan/input-plane-strategy branch 2 times, most recently from a998493 to 48296fb Compare June 18, 2025 18:34

rculbertson force-pushed the ryan/control-plane-strategy branch from 75afd74 to 55e27f0 Compare June 18, 2025 18:36

rculbertson force-pushed the ryan/input-plane-strategy branch from 48296fb to 02763e7 Compare June 18, 2025 18:37

rculbertson requested a review from ekzhang June 18, 2025 18:37

rculbertson force-pushed the ryan/control-plane-strategy branch from 55e27f0 to eb9fcc6 Compare June 18, 2025 18:56

rculbertson force-pushed the ryan/input-plane-strategy branch from 02763e7 to 7bc2d75 Compare June 18, 2025 18:56

rculbertson force-pushed the ryan/control-plane-strategy branch 2 times, most recently from a999ab3 to 99f9f77 Compare June 18, 2025 19:43

rculbertson force-pushed the ryan/input-plane-strategy branch from 7bc2d75 to 14f9d4f Compare June 18, 2025 20:06

rculbertson force-pushed the ryan/control-plane-strategy branch from a38d9c2 to 90b362b Compare June 18, 2025 20:24

rculbertson force-pushed the ryan/input-plane-strategy branch from 14f9d4f to 56efc9b Compare June 19, 2025 20:35

Base automatically changed from ryan/control-plane-strategy to main June 20, 2025 15:06

rculbertson force-pushed the ryan/input-plane-strategy branch 4 times, most recently from 4d45866 to 23652f8 Compare June 20, 2025 15:44

ekzhang reviewed Jun 26, 2025

View reviewed changes

rculbertson mentioned this pull request Jun 30, 2025

Add support for retrying internal failures #30

Closed

rculbertson mentioned this pull request Jun 30, 2025

Add support for calling input plane functions #15

Closed

rculbertson force-pushed the ryan/input-plane-strategy branch 3 times, most recently from c3bcd2d to 90673b6 Compare June 30, 2025 15:54

rculbertson requested review from ehdr and ekzhang June 30, 2025 20:52

rculbertson force-pushed the ryan/input-plane-strategy branch from f1f43a9 to 9fff513 Compare June 30, 2025 20:54

Add support for input plane

a7b097b

rculbertson force-pushed the ryan/input-plane-strategy branch 3 times, most recently from 1315218 to 3635731 Compare June 30, 2025 22:15

Address PR review comments

c8fe4fb

rculbertson force-pushed the ryan/input-plane-strategy branch from 3635731 to c8fe4fb Compare June 30, 2025 22:17

ehdr approved these changes Jul 2, 2025

View reviewed changes

Remove catch

1b26949

rculbertson force-pushed the ryan/input-plane-strategy branch from 79ff102 to 1b26949 Compare July 2, 2025 14:09

rculbertson merged commit 94d225f into main Jul 2, 2025
5 checks passed

rculbertson deleted the ryan/input-plane-strategy branch July 2, 2025 14:14

billyb2 pushed a commit to depot/libmodal that referenced this pull request Jul 16, 2025

Add support for input plane invocation (modal-labs#32)

fde1452

* Add support for input plane * Address PR review comments * Remove catch

Conversation

rculbertson commented Jun 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rculbertson commented Jun 20, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rculbertson commented Jun 30, 2025

Uh oh!

ehdr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rculbertson commented Jul 2, 2025

Uh oh!

Uh oh!

ekzhang commented Jul 2, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rculbertson commented Jun 17, 2025 •

edited

Loading