-
Notifications
You must be signed in to change notification settings - Fork 167
feat: integrate cloudevents module #160
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.
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
Conversation
Signed-off-by: Grant Timmerman <[email protected]>
Signed-off-by: Grant Timmerman <[email protected]>
There might be some issues to address before landing this. Current errors:
|
@grant consider upgrading your typescript dependency. In the cloudevents module, we are using 3.8.3 and with that version |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Assuming typescript is updated and the presubmit checks pass.
Merge with master should pull the latest TS version now. |
I think there needs to be some work with fixing the invoker for the current CE implementation before adding this module. I've been getting into feature-creep trying to fix too many things at once. So I'll close this PR and we should continue with a smaller PR. |
Integrates the CloudEvents SDK for the Node Functions Framework. Does not change functionality.
cloudevents
moduleevent
signature. (This signature should not receive or support CloudEvents)Potential Improvements
Production Impact
Should not have impact on GCF invoker as:
wrapCloudEventFunction
in GCFisBinaryCloudEvent
resolving totrue
inwrapEventFunction
cloudevents
bundle size is reasonable at <200kB: https://bundlephobia.com/[email protected]CC: @lance @grayside