-
Notifications
You must be signed in to change notification settings - Fork 3
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
💝 Event receiver & stream via Server-Sent Events #11
💝 Event receiver & stream via Server-Sent Events #11
Conversation
c5aaa4b
to
f223aab
Compare
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.
A self-review 😼
quarkus/src/main/java/com/redhat/openshift/knative/showcase/events/EventStore.java
Show resolved
Hide resolved
quarkus/src/main/java/com/redhat/openshift/knative/showcase/events/Endpoint.java
Outdated
Show resolved
Hide resolved
quarkus/src/main/java/com/redhat/openshift/knative/showcase/events/Endpoint.java
Outdated
Show resolved
Hide resolved
/cc @mgencur |
5a4a52a
to
c9a0fe2
Compare
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.
Hi @cardil , thanks for implementing the new features. I have only tested one part of it (the frontend which sends evens repeatedly) but I have a left a number of comments.
This application is named "OpenShift Knative Showcase" or "Knative Showcase" and it mentions Knative in many places. It also says it "showcase the Knative features".
For me, this application shows sending and receiving CloudEvents in a nice way. It can be used side-by-side with Knative Eventing to show events that are sent/received. But it doesn't have/show Knative features. So, mentioning "Knative" all over the place is misleading at this point.
I don't know what will be the future of this application. Do you have any plans for functionality that would demonstrate Knative features?
Thanks
quarkus/src/main/java/com/redhat/openshift/knative/showcase/events/EventStore.java
Show resolved
Hide resolved
Well, this frontend app, by in development mode, by default work on stubs. Those stubs simulate receiving events continuously. But, that's just a simulation.
The eventing bits (event receive, and streaming) are just coming in this PR. This app already has a number of features that's helpful to showcase Serving functionality. Have you seen the README? Especially after the changes in this PR (a little cleanup). There are a couple of things that help showcase Serving:
Also, already in, is the sending of the event to the So, I think this app already helps showcase Knative features, and this PR only adds to that. |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cardil, mgencur The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Fixes SRVCOM-2287
Changes
Explanation
The UI was extracted from both backends (they had used server-side rendering before), into a separate React app written in Typescript.
This React app is now built into a set of static files, which are packaged as a webjar for easy consumption on both backends (especially in the Java one). The frontend application, of course, can be started in development mode, either running on stubs, or connecting to the real backend.
Both backends have the event receiver implemented at
POST /events
. Both backends have the Server-Sent Events (SSE) endpoint implemented atGET /events
. You can connect to it in streaming mode using tools like HTTPie (the connection will be left hanging, and any new events will be delivered to the client, CloudEvents are encoded as structured):The React frontend have also the dedicated UI component that connects to that event stream and presents it in a nice table:
Related issues
During the developments I've stumbled on a number of upstream issues, I needed to work around:
SseUtil
unexpectedly stores headers inSerialisers.EMPTY_MULTI_MAP
quarkusio/quarkus#31559MessageBodyWriter
quarkusio/quarkus#31587/kind enhancement