Skip to content

Conversation

@taeold
Copy link
Contributor

@taeold taeold commented Nov 9, 2021

We introduce a bin associated with the firebase-functions SDK! The binary exposes a simple server on localhost that describes the Firebase Functions defined in the current directory:

$ STACK_CONTROL_API_PORT=8181 npx firebase-functions # Optionally can pass in a FUNCTIONS_DIR as first arg Serving at port 8181 $ curl localhost:8181/__/stack.yaml | jq { "endpoints": { "onreqv2": { "platform": "gcfv2", "labels": {}, "httpsTrigger": {}, "entryPoint": "onreqv2" }, "onRequest": { "platform": "gcfv1", "httpsTrigger": {}, "entryPoint": "onRequest" } }, "specVersion": "v1alpha1", "requiredAPIs": [] } 
@google-cla google-cla bot added the cla: yes label Nov 9, 2021
@taeold taeold changed the base branch from dl-container-contract to dl-endpoint-prop November 9, 2021 23:16
@taeold taeold requested a review from inlined November 9, 2021 23:30
Copy link
Member

@inlined inlined left a comment

Choose a reason for hiding this comment

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

I like it, but let's get the port/Env var + the requiredAPIs stuff sorted out.

All in all though, this is much better than I could have done myself. We're really lucky that you've got experience with ESM modules vs pacakges.

taeold and others added 4 commits December 9, 2021 21:18
Follows up #999 to annotate each funuctions with `__endpoint` property. Highlight of changes: * Extend unit test coverage for all v1 providers * Add `__endpoint` annotation to v1 task queue functions * Add `__requiredAPIs` annotation to task queue and scheduler functions * Explicitly set `__endpoint` to undefined in the handler namespace * No SDK-level label setting in the __endpoint annotation.
@taeold taeold changed the base branch from dl-endpoint-prop to dl-container-contract December 10, 2021 05:39
@taeold taeold requested a review from colerogers December 10, 2021 05:54
@taeold taeold marked this pull request as ready for review December 10, 2021 19:29
@taeold taeold requested a review from inlined December 14, 2021 06:46
Copy link
Contributor

@colerogers colerogers left a comment

Choose a reason for hiding this comment

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

looks good, just one thing that I had a question on

Base automatically changed from dl-container-contract to master January 19, 2022 18:18
Copy link
Member

@inlined inlined left a comment

Choose a reason for hiding this comment

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

Still looking good.

const stack = await loadStack(functionsDir);
res.setHeader('content-type', 'text/yaml');
res.send(JSON.stringify(backend));
res.send(JSON.stringify(stack));
Copy link
Member

Choose a reason for hiding this comment

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

I mean.... technically JSON is YAML?

},
{
api: 'cloudscheduler.googleapis.com',
reason: 'Needed for v1 scheduled functions.',
Copy link
Member

Choose a reason for hiding this comment

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

here's a case where I could argue for using "v1" because v2 will not use Pub/Sub.

Alternatively you can just remove this required API because it's part of the APIs that get enabled with Cloud Functions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think Cloud Scheduler gets enabled as part of GCF, but I'm not sure what enables it since I do see it enabled when creating a new GCP project 🤔 . I'm going to add 'v1' to err on side of being safe, but can remove later when I dig in and figure out if we can rely on the cloud scheduler API being enabled on users project by some means.

@taeold taeold merged commit f3ed261 into master Feb 2, 2022
@taeold taeold deleted the dl-ff-runtime branch February 2, 2022 13:21
taeold added a commit that referenced this pull request Feb 7, 2022
…#1033) Follows up #1032 for V2 API to fix #1031. While rebasing the branch that moved the location of manifest definition (#1003), the original file was not removed and cleaned up. Having long-lasting PR is dangeroud 🤦🏼‍♂️ .
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3 participants