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.

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

Added new section in README.md to clarify how to do local testing of … #40

Closed
wants to merge 2 commits into from

Conversation

comolongo
Copy link

Added new section in README.md to clarify how to do local testing of event functions. Fixes the questions raised in issue #37.

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@stew-r
Copy link
Contributor

stew-r commented Jun 18, 2019

This is a great contribution -- thank you!

@ace-n @grant think we can have this exist as a separate testing doc?

@comolongo have you tested whether this actually works with the Pub/Sub emulator? I think we have an issue right now where Pub/Sub messages are transformed in our infrastructure before reaching the container with your function. This means that the framework expects a slightly different message format.

To be clear: this is a bug that we need to fix. We just haven't gotten around to it yet.

@grant
Copy link
Contributor

grant commented Jun 18, 2019

@stew-r I recommend that we have a docs/ folder in this repo for advanced documentation.

I can create that:
#42
Advanced guides should talk about things like using the emulator, multiple functions, etc.

As we gain more options for using the Functions Framework, this folder can help support docs.

@comolongo
Copy link
Author

Hey @stew-r , yes I have tested this with the PubSub emulator, and it does have the issues you're talking about. I was able to overcome it, by hacking the wrapEventFunction() in invoker.ts and filed ticket #41 to address that specifically.

On the topic of the PubSub emulator incorrectly transforming the messages, at least from my tests, it seems like the emulator is pushing notifications using the same format as on prod - i.e. it seems like the actual body of their push notification is correct. The issue may be that the emulator needs to include the right header flags for invoker::isBinaryCloudEvent() to return true. Currently it is returning false, and causing wrapEventFunction() to go down the wrong else clause.

@stew-r
Copy link
Contributor

stew-r commented Jun 18, 2019

@grant I like the structure -- thanks!

@comolongo so I think the issue is somewhat different (though your solution addresses it too). In short:

  • Pub/Sub sends a message
  • In Google Cloud Functions, we have a piece of infrastructure that receives the message and applies some transformation before passing it on to the user's container
  • The user's container thus doesn't see a Pub/Sub message but a transformed Pub/Sub message

When running locally (or a different container-based environment), we don't perform this transformation. So your function sees something different based on whether it is running in Google Cloud Functions vs. elsewhere.

As you've discovered, this is solvable. Thanks for submitting a fix, I'll make sure to pass it along to the eng team.

@grant grant mentioned this pull request Jun 24, 2019
@grant
Copy link
Contributor

grant commented Jun 27, 2019

@comolongo Could you change this PR to add this content into the docs folder?

@ghost
Copy link

ghost commented Jun 30, 2019

Might be handy for typescript users if it is stated that the entry file is dictated by your "main": "./lib/index.js", property in package.json?

It just seemed a little odd to me that I could not add an argument to the command, most testing CLIs follow that flow.

@grant
Copy link
Contributor

grant commented Jul 9, 2019

@comolongo Could you move markdown content into the docs/ folder?

I'm trying to resolve PRs within 30 days. If there's no reply, we should add the guides outside of this PR.


The file can be in any folder on your computer. From the terminal, goto the directory where ```mockPubsub.json``` is located, and run the following command assuming your cloud function is hosted locally on port 8080:
```sh
curl -d "@mockPubsub.json" -X POST http://localhost:8080
Copy link
Contributor

Choose a reason for hiding this comment

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

May I also suggest to add the CE-* headers, maybe with a truthy value, to the docs so the lib can know that the request is for a Cloud Event?

curl -d "@mockPubsub.json" -X POST -H "Ce-Type: true" -H "Ce-Specversion: true" -H "Ce-Source: true" -H "Ce-Id: true" http://localhost:8080

This would help with issues like #41, and also set the values correctly to the data and context arguments.

@grant
Copy link
Contributor

grant commented Aug 1, 2019

Hey @comolongo,
Can you move the change here to the docs/ folder? I'll merge this right after.

@grant grant closed this Aug 16, 2019
@grant
Copy link
Contributor

grant commented Aug 16, 2019

Closed due to inactivity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants