Pass on HTTP Path when calling functions asynchronously

jpauwels commented 5 years ago

This commit fixes faas issue https://github.com/openfaas/faas/issues/1035. It requires the vendored faas queue.Request struct to be updated to the latest version in the faas repo.

Signed-off-by: Johan Pauwels jpauwels@users.noreply.github.com

Description

Motivation and Context

[x] I have raised an issue to propose this change (required)

How Has This Been Tested?

Local (macOS) and server (Ubuntu) deployment with Docker Swarm.

Types of changes

[ ] Bug fix (non-breaking change which fixes an issue)
[x] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

[x] My code follows the code style of this project.
[x] My change requires a change to the documentation.
[ ] I have updated the documentation accordingly.
[x] I've read the CONTRIBUTION guide
[x] I have signed-off my commits with git commit -s
[ ] I have added tests to cover my changes.
[ ] All new and existing tests passed.

alexellis commented 5 years ago

Hi Johan thank you for working on this.

Please could you show end-to-end testing using the following? https://github.com/alexellis/tensorflow-serving-openfaas

Thanks,

Alex

jpauwels commented 5 years ago

I've cloned your TS serving repo and can replicate the given example out of the box. Are you interested in any particular log files?

jpauwels commented 5 years ago

Here's the requestbin: https://requestbin.fullcontact.com/142ob7k1?inspect

alexellis commented 5 years ago

That's great

alexellis commented 5 years ago

Hi @jpauwels a real email address is required for the sign-off.

Signed-off-by: Johan Pauwels [jpauwels@users.noreply.github.com](mailto:jpauwels@users.noreply.github.com)

This should be your qmul or a personal address. Please amend with:

git config user.name
git commit -s --amend

Thanks

jpauwels commented 5 years ago

Added email and rebased

alexellis commented 5 years ago

So do we just need a new release of the queue worker and that is it?

jpauwels commented 5 years ago

So do we just need a new release of the queue worker and that is it?

Indeed

jpauwels commented 5 years ago

So do we just need a new release of the queue worker and that is it?

The necessary work in the main repo has been done by @telackey in https://github.com/openfaas/faas/commit/decf9addb37bec43e1ae7b91fb860837671597b6 six months ago. He might be able to tell you what the motivation was for those changes.

alexellis commented 5 years ago

I worked on those changes too and actually finished the patch. If you can find the old PRs you'll see it. I'm just asking you what packaging you think we need to do as a result of your change?

jpauwels commented 5 years ago

Even better then. So yes, just release a new version of the queue worker and change the docker-compose.yml in the main repo and that should be it.

alexellis commented 5 years ago

I'll cut a new release. Would you be able to do the updates for docker swarm and for Kubernetes? if you have any questions I'm sure people would be more than willing to help on Slack.

jpauwels commented 5 years ago

Looks like it's being done, which is good cause I don't really know what you mean by that. I've never used Kubernetes btw and my Swarm just consists of a single node.

openfaas / nats-queue-worker