-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Funqy Google Cloud Function #9655
Funqy Google Cloud Function #9655
Conversation
/cc @patriot1burke |
1a78986
to
a8c0702
Compare
a8c0702
to
88ee679
Compare
@loicmathieu Please see: #9717 Had to change the VirtualChannel implementation so that FileRegions could be supported (static content). |
@patriot1burke thanks for the heads up on #9717, I'll update this PR when the code is merged. |
d00ac87
to
d6f2cc4
Compare
@patriot1burke this PR is ready to review. |
|
||
import io.quarkus.runtime.Application; | ||
|
||
public class FunqyHttpFunction implements HttpFunction { |
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.
We is there a specific need for Funqy + HTTP + GCP? Isn't this handled by a more generic function?
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.
GCF have 3 distinct function interface, one for HttpFunction and two for BackgroundFunction. So I use two different implementation.
We can mix the two function inside he same implementation if you think it's easier for the user to use and easier for us to maintain.
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.
I'm saying the work you already did that is merged for GCP + HTTP will work with Funqy already. If your extension works for Servlet, JAX-RS, and Vert.x Web, it will work for Funqy. There's no need for a new funqy+http+gcp extension.
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.
OK, so I need to update the GCF HTTP support to support Funqy right ?
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.
No, GCF HTTP wont' have to do anything to support Funqy. That's my point. We only have quarkus-funqy-amazon-lambda because raw lambda does not use HTTP. Notice that we don't have quarkus-funqy-azure-functions either.
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.
If I compare whith what you provides for Amazon, Funqy HTTP for GCF should be using both funqy-http
and google-cloud-http
.
If it's the case, I need to test it and, if working, just provides documentation.
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.
Yes exactly: funqy-http + google-cloud-http. No need for a funqy-gcf-http layer.
Also, what about my other question? If you have funqy integrated with gcf http, why would you want background implementation? I can understand pub/sub, but what does background provide?
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.
OK, I'll remove it and document the fact that funqy-http will work with google-cloud-functions-http.
Background is the generic term for not-http function, it is triggered by any event that Google Cloud support (pubsub, storage, firestore, ...).
So this is why we need Funqy for.
I give examples for PubSub and Storage, on the example I show how you can trigger them via HTTP for testing purpose (via the local invoker) but on GCP this will not be triggered via HTTP so we need Funqy for it.
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.
HttpFunction binding removed, we can know call only background function via Funqy (so only respond to Cloud events, not to HTTP trigger).
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.
@patriot1burke is it OK now that I remove the HttpFunction binding ?
integration-tests/funqy-google-cloud-functions/src/main/resources/application.properties
Outdated
Show resolved
Hide resolved
...sts/funqy-amazon-lambda/src/test/java/io/quarkus/funqy/gcp/functions/test/GreetTestBase.java
Outdated
Show resolved
Hide resolved
80e797c
to
ee3f10b
Compare
19e9bde
to
1dbdb8d
Compare
@patriot1burke did you have time to look at this PR? 1.6 will be cut soon and it would be cool to have the multiple Google Cloud extensions in it. |
3950856
to
6c3a01b
Compare
3901795
to
7aaf22d
Compare
7aaf22d
to
7ae62d0
Compare
@patriot1burke did you had time to look into this PR ? |
Funqy binding for Google Cloud Functions.
Allow both Http function and Background Function (PubSub, Storage, Firestore, ...).
I created a draft PR because there is still some work to be done: integration test, maybe native ...