fix: Allow registering multiple functions with one server for local testing. #143
Conversation
jihuin
changed the title
| @@ -39,10 +39,6 @@ const ( | |||
| fnErrorMessageStderrTmpl = "Function error: %v" | |||
| ) | |||
|
|
|||
| var ( | |||
| handler http.Handler | |||
funcframework/framework.go
Outdated
| if err == nil { | ||
| handler = server | ||
| if err := registry.Default().RegisterHTTP(path, fn, registry.WithPath(path)); err != nil { | ||
| return fmt.Errorf("failed to register function at path %s: %s", path, err) |
There was a problem hiding this comment.
[nit] maybe include function target in the error message too? Also, prefer %q for strings instead of %s
There was a problem hiding this comment.
The function name is included so it should be fine. Changed %s to %q
funcframework/framework.go
Outdated
| server, err := wrapHTTPFunction(path, fn) | ||
| if err == nil { | ||
| handler = server | ||
| if err := registry.Default().RegisterHTTP(path, fn, registry.WithPath(path)); err != nil { |
There was a problem hiding this comment.
Hmm, maybe just return registry.Default().RegisterHTTP's result directly? Make the error message returned by that more detailed if it isn't detailed enough.
internal/registry/registry.go
Outdated
| EventFn interface{} // Optional: The user's Event function | ||
| } | ||
|
|
||
| // Option is |
There was a problem hiding this comment.
Done. Thanks for catching this.
funcframework/framework.go
Outdated
| server, err := wrapHTTPFunction(path, fn) | ||
| if err == nil { | ||
| handler = server | ||
| if err := registry.Default().RegisterHTTP(path, fn, registry.WithPath(path)); err != nil { |
There was a problem hiding this comment.
Oh I kind of forgot these didn't have names... Maybe we should give it a readable name like fmt.Sprintf("function at path %q", path) or something so that error messages that print the function name won't look as weird.
|
|
||
| if err := RegisterHTTPFunctionContext(context.Background(), "/", func(w http.ResponseWriter, r *http.Request) { |
There was a problem hiding this comment.
I think we still want to have some test that mixes both the older RegisterHTTPFunctionContext and functions.HTTP calls and makes sure things work as expected.
… local testing. (GoogleCloudPlatform#143)" This reverts commit 3cab285.
…testing (#154) * feat: Allow registering multiple functions with one server for local testing (restore PR #143) * feat: serve the last func that's not registered declaratively if no target is found * Store declaratively defined functions in map and others in array * update error message * Add test case that multiple options are specified * update registry reset func * change the implementation of registry reset func
There are two ways to test multiple functions within one server.
functions.HTTP()) and rungo run cmd/main.gowithout setting the env varFUNCTION_TARGET. In this case, all registered functions will be served at "/{func_name}".funcframework.RegisterHTTPFunctionContext()) and rungo run cmd/main.gowithout setting the env varFUNCTION_TARGET. In this case, the registered handlers will be served at the user defined path.Closes #109