Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: allow --unstable flag in functions serve #1139

Closed
wants to merge 2 commits into from
Closed

feat: allow --unstable flag in functions serve #1139

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5c7b12d
feat: allow --unstable flag in functions serve
sammccord May 23, 2023
321a341
Merge branch 'main' into allow-unstable-flag
sammccord May 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion cmd/functions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ var (
}

envFilePath string
unstable = new(bool)
sweatybridge marked this conversation as resolved.
Show resolved Hide resolved

functionsServeCmd = &cobra.Command{
Use: "serve <Function name>",
Expand All @@ -95,7 +96,7 @@ var (
if len(args) > 0 {
slug = args[0]
}
return serve.Run(ctx, slug, envFilePath, noVerifyJWT, importMapPath, afero.NewOsFs())
return serve.Run(ctx, slug, envFilePath, noVerifyJWT, importMapPath, unstable, afero.NewOsFs())
},
}
)
Expand All @@ -111,6 +112,7 @@ func init() {
functionsServeCmd.Flags().StringVar(&envFilePath, "env-file", "", "Path to an env file to be populated to the Function environment.")
functionsServeCmd.Flags().StringVar(&importMapPath, "import-map", "", "Path to import map file.")
functionsServeCmd.Flags().Bool("all", true, "Serve all Functions (caution: experimental feature)")
functionsServeCmd.Flags().BoolVar(unstable, "unstable", false, "Enable the use of unstable Deno APIs")
cobra.CheckErr(functionsServeCmd.Flags().MarkHidden("all"))
functionsDownloadCmd.Flags().StringVar(&flags.ProjectRef, "project-ref", "", "Project ref of the Supabase project.")
functionsCmd.AddCommand(functionsDeleteCmd)
Expand Down
7 changes: 6 additions & 1 deletion internal/functions/serve/serve.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ var (
mainFuncEmbed string
)

func Run(ctx context.Context, slug string, envFilePath string, noVerifyJWT *bool, importMapPath string, fsys afero.Fs) error {
func Run(ctx context.Context, slug string, envFilePath string, noVerifyJWT *bool, importMapPath string, unstable *bool, fsys afero.Fs) error {
if len(slug) == 0 {
return runServeAll(ctx, envFilePath, noVerifyJWT, importMapPath, fsys)
}
Expand Down Expand Up @@ -174,6 +174,11 @@ func Run(ctx context.Context, slug string, envFilePath string, noVerifyJWT *bool
} else {
return fmt.Errorf("failed to check index.ts for function %s: %w", slug, err)
}

if *unstable {
denoRunCmd = append(denoRunCmd, "--unstable")
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you tried deploying the puppeteer example with supabase functions deploy? If that works, we can always add the --unstable flag to deno run by default.

Ideally the same code should work in both local serving and production deployment.

Copy link
Author

@sammccord sammccord May 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

For probably unrelated reasons, the puppeteer example doesn't work locally all for me (even on official cli), there's some issue with readable streams that doesn't occur when running the same with deno run -

Error: channel closed
    at async UserWorker.fetch (ext:sb_user_workers/user_workers.js:54:21)
    at async Server.<anonymous> (file:///home/deno/main/index.ts:104:16)
    at async Server.#respond (https://deno.land/[email protected]/http/server.ts:220:24)
worker thread panicked Uncaught SyntaxError: The requested module '/v124/[email protected]/esnext/lib/readable-stream-browser.js' does not provide an export named 'default'
    at https://esm.sh/v124/[email protected]/esnext/jszip.mjs:2:291
worker "/home/deno/functions/puppeteer" returned an error: Uncaught SyntaxError: The requested module '/v124/[email protected]/esnext/lib/readable-stream-browser.js' does not provide an export named 'default'
    at https://esm.sh/v124/[email protected]/esnext/jszip.mjs:2:291

... but since it works great with deno deploy - https://gqlatxhpnxwuzspuqqvh.functions.supabase.co/puppeteer, and deno bundle and deno run both support --unstable, maybe it could be the default behavior? I just really want to play with Deno KV!

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm afraid Deno KV is not currently supported by supabase functions because we are switching to edge-runtime for both serving locally and prod deployment. The --unstable flag added in this PR targets the legacy deno relay server which we have deprecated.

Please consider opening a github issue with edge runtime repo to help our functions team prioritise Deno KV support.

}

denoRunCmd = append(denoRunCmd, dockerFuncPath)
}

Expand Down
12 changes: 6 additions & 6 deletions internal/functions/serve/serve_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func TestServeCommand(t *testing.T) {
Post("/v" + utils.Docker.ClientVersion() + "/containers").
Reply(http.StatusServiceUnavailable)
// Run test
err := Run(context.Background(), "test-func", "", nil, "", fsys)
err := Run(context.Background(), "test-func", "", nil, "", nil, fsys)
// Check error
assert.ErrorContains(t, err, "request returned Service Unavailable for API route and version")
assert.Empty(t, apitest.ListUnmatchedRequests())
Expand All @@ -63,7 +63,7 @@ func TestServeCommand(t *testing.T) {
require.NoError(t, apitest.MockDockerLogs(utils.Docker, containerId, "success"))
// Run test
noVerifyJWT := true
err := Run(context.Background(), "", ".env", &noVerifyJWT, "", fsys)
err := Run(context.Background(), "", ".env", &noVerifyJWT, "", nil, fsys)
// Check error
assert.NoError(t, err)
assert.Empty(t, apitest.ListUnmatchedRequests())
Expand All @@ -73,7 +73,7 @@ func TestServeCommand(t *testing.T) {
// Setup in-memory fs
fsys := afero.NewMemMapFs()
// Run test
err := Run(context.Background(), "", "", nil, "", fsys)
err := Run(context.Background(), "", "", nil, "", nil, fsys)
// Check error
assert.ErrorContains(t, err, "open supabase/config.toml: file does not exist")
})
Expand All @@ -89,7 +89,7 @@ func TestServeCommand(t *testing.T) {
Get("/v" + utils.Docker.ClientVersion() + "/containers/supabase_db_test/json").
ReplyError(errors.New("network error"))
// Run test
err := Run(context.Background(), "", "", nil, "", fsys)
err := Run(context.Background(), "", "", nil, "", nil, fsys)
// Check error
assert.ErrorIs(t, err, utils.ErrNotRunning)
})
Expand All @@ -106,7 +106,7 @@ func TestServeCommand(t *testing.T) {
Reply(http.StatusOK).
JSON(types.ContainerJSON{})
// Run test
err := Run(context.Background(), "", ".env", nil, "", fsys)
err := Run(context.Background(), "", ".env", nil, "", nil, fsys)
// Check error
assert.ErrorContains(t, err, "open .env: file does not exist")
})
Expand All @@ -124,7 +124,7 @@ func TestServeCommand(t *testing.T) {
Reply(http.StatusOK).
JSON(types.ContainerJSON{})
// Run test
err := Run(context.Background(), "", ".env", nil, "import_map.json", fsys)
err := Run(context.Background(), "", ".env", nil, "import_map.json", nil, fsys)
// Check error
assert.ErrorContains(t, err, "Failed to read import map")
assert.ErrorContains(t, err, "file does not exist")
Expand Down