Skip to content

Knative related improvements#152

Closed
lburgazzoli wants to merge 2 commits intoapache:masterfrom
lburgazzoli:github-151
Closed

Knative related improvements#152
lburgazzoli wants to merge 2 commits intoapache:masterfrom
lburgazzoli:github-151

Conversation

@lburgazzoli
Copy link
Copy Markdown
Contributor

@lburgazzoli lburgazzoli commented Oct 9, 2019

Fixes #151
Fixes #150

@lburgazzoli lburgazzoli changed the title Support Knative broker/trigger model Knative related improvements Oct 9, 2019
Copy link
Copy Markdown
Member

@nicolaferraro nicolaferraro left a comment

Choose a reason for hiding this comment

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

Everything looks good. I'd only allow event to act as a producer too, to publish an event, i.e. sending it to the broker.

final String version = configuration.getCloudEventsSpecVersion();
final Processor ceProcessor = CloudEventsProcessors.forSpecversion(version).producerProcessor(this);
if (type == Knative.Type.event) {
throw new UnsupportedOperationException("knative `events` are supported only as consumer");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Mmh, why? I should be allowed to send events to the broker mesh

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It should behave as any other endpoint or channel, so no code changes

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

so the event type is also used to lookup the service definition from the knative environment json ?

Copy link
Copy Markdown
Member

@nicolaferraro nicolaferraro Oct 10, 2019

Choose a reason for hiding this comment

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

No.. producer side the event type can be used to set the value of the ce-type header (when not overridden by the route headers).

I'm just thinking a possible way to define it..

.to('knative:event/chuck')

Semantically may mean "transform the exchange into a chuck event and emit it".

Destination is always the "default" broker unless overridden. So we may expect at runtime level that the operator will inject the destination into the KNATIVE config and we need to look it up here (by type and name, where name is default if not overridden).

@lburgazzoli
Copy link
Copy Markdown
Contributor Author

reworking

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support Knative broker/trigger model Support explicit source/target in knative endpoint

2 participants