Skip to content

Adding docs/spec shell.#147

Closed
n3wscott wants to merge 7 commits into
knative:masterfrom
n3wscott:spec
Closed

Adding docs/spec shell.#147
n3wscott wants to merge 7 commits into
knative:masterfrom
n3wscott:spec

Conversation

@n3wscott
Copy link
Copy Markdown
Contributor

The serving/docs/spec dir is an awesome resource. Eventing should have one too.

@google-prow-robot google-prow-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jun 29, 2018
@n3wscott
Copy link
Copy Markdown
Contributor Author

/assign mattmoor

@google-prow-robot google-prow-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jun 29, 2018
Copy link
Copy Markdown
Member

@evankanderson evankanderson left a comment

Choose a reason for hiding this comment

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

More comments later, have several hours of meetings next.

Comment thread docs/spec/README.md Outdated

This directory contains the specification of the Knative Eventing API, which is
implemented in [`pkg/apis/channels/v1alpha1`](/pkg/apis/channels/v1alpha1) and
[`pkg/apis/feeds/v1alpha1`](/pkg/apis/feeds/v1alpha1) and verified via [the e2e
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.

Prefer to specify the k8s apiversion paths, rather than the repo paths.

Comment thread docs/spec/motivation.md Outdated
# Motivation

The goal of the Knative Eventing project is to provide a common toolkit and API
framework for producing and consuming events, primarily inside of serverless
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.

I'd stress that this is intended to complement serverless workloads, but should work independently of any particular serverless implementation.

It's probably also worth referencing the CloudEvents spec.

Feel free to also lift text from the Eventing Glossary.

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.

ok, linked and reworded. Left off the serverless aspect.

Copy link
Copy Markdown
Member

@evankanderson evankanderson left a comment

Choose a reason for hiding this comment

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

More notes, almost done.

I may send you a PR or two as well.

Comment thread docs/spec/motivation.md Outdated
workloads.

Kubernetes has no primitives related to event processing, yet this is an
essential component in serverless workloads. Eventing will introduce high-level
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.

essential component (citation needed)

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.

I don't have a citation... hm

Comment thread docs/spec/motivation.md Outdated
workloads.

Kubernetes has no primitives related to event processing, yet this is an
essential component in serverless workloads. Eventing will introduce high-level
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.

Prefer active voice "Eventing introduces"

Comment thread docs/spec/motivation.md Outdated
integrate with existing message consumers.

The Knative Eventing APIs consist of Eventing API (these documents),
[Compute API](https://github.com/knative/serving) and
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.

Eventing should be able to operate independently of serving and build, but those may be the best targets for eventing delivery (in terms of cost & observability).

Comment thread docs/spec/motivation.md Outdated

Kubernetes has no primitives related to event processing, yet this is an
essential component in serverless workloads. Eventing will introduce high-level
primitives for event production and delivery in forms that are extensible. If a
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.

We should scope the extensibility of production and delivery. My personal $0.02 given the architecture of knative/serving and Istio is that event delivery should be push (retried if needed), and that delivery should be done using HTTP-compatible framing (because that works nearly everywhere), and provides nice mechanisms for multiplexing.

Comment thread docs/spec/overview.md
@@ -0,0 +1,61 @@
# Resource Groups

Knative Eventing API is grouped into _Channels_ and _Feeds_:
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.

Slight preference for "Delivery" over "Channels", but @scothis may have sharper opinions than I.

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.

I am just basing this on the k8s grouping currently being used in the CRDs

Comment thread docs/spec/overview.md Outdated

The primary resources in the Knative Eventing _Feeds_ API are EventSource, EventType and Bind:

* An **EventSource** wraps an event producer and is able to produce multiple definitions of
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.

Slight preference for standardizing on terms in the Eventing Glossary and/or in the CloudEvents spec. If the current names are terrible, propose new ones in the glossary as well as here.

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.

I am attempting to document what is implemented. I have tried to suggest changes to the glossary to bring it closer to what has been implemented.

Comment thread docs/spec/overview.md Outdated

The primary resources in the Knative Eventing _Feeds_ API are EventSource, EventType and Bind:

* An **EventSource** wraps an event producer and is able to produce multiple definitions of
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.

The end of this sentence got lost.

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.

I was following the serving/docs/spec overview where they string the definitions of each type into a paragraph flow...

Comment thread docs/spec/overview.md Outdated

The primary resources in the Knative Eventing _Feeds_ API are EventSource, EventType and Bind:

* An **EventSource** wraps an event producer and is able to produce multiple definitions of
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.

Thoughts on calling this just a "Source"?

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.

EventSource.feeds.knative.dev vs Source.feeds.knative.dev

hmm... I do like that EventType matches, because these are tied together.

EventType.feeds.knative.dev

But type is a reserved word in golang.

Cloudevents uses eventType and source. so... I am in favor of changing to Source.

Comment thread docs/spec/overview.md Outdated

* An **EventSource** wraps an event producer and is able to produce multiple definitions of

* **EventType**, which describes the schema for the shape of the event and Subscription.
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.

s/Subscription/Bind/ ?

Comment thread docs/spec/overview.md Outdated

The primary resources in the Knative Eventing _Channels_ API are Channel, Subscription and Bus:

* A **Channel** is a logical endpoint to allow events to flow into a
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.

I'd prefer to see this be a complete sentence. Bonus points if the object reference graph is acyclic, but that may not be possible.

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.

I think it would be more clear as real sentences too. Let me just rework it and not try to copy the style of serving.

@google-prow-robot google-prow-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 2, 2018
Copy link
Copy Markdown
Contributor Author

@n3wscott n3wscott left a comment

Choose a reason for hiding this comment

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

I moved over the glossary terms here, I think it needs more work and reading the glossary showed me that there might be some concepts that are not backed by a CR directly, but are properties in those CRs.

Comment thread docs/spec/motivation.md Outdated
workloads.

Kubernetes has no primitives related to event processing, yet this is an
essential component in serverless workloads. Eventing will introduce high-level
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.

I don't have a citation... hm

Comment thread docs/spec/motivation.md Outdated
# Motivation

The goal of the Knative Eventing project is to provide a common toolkit and API
framework for producing and consuming events, primarily inside of serverless
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.

ok, linked and reworded. Left off the serverless aspect.

Comment thread docs/spec/overview.md
@@ -0,0 +1,61 @@
# Resource Groups

Knative Eventing API is grouped into _Channels_ and _Feeds_:
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.

I am just basing this on the k8s grouping currently being used in the CRDs

Comment thread docs/spec/overview.md Outdated

The primary resources in the Knative Eventing _Feeds_ API are EventSource, EventType and Bind:

* An **EventSource** wraps an event producer and is able to produce multiple definitions of
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.

I was following the serving/docs/spec overview where they string the definitions of each type into a paragraph flow...

Comment thread docs/spec/overview.md Outdated

The primary resources in the Knative Eventing _Feeds_ API are EventSource, EventType and Bind:

* An **EventSource** wraps an event producer and is able to produce multiple definitions of
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.

I am attempting to document what is implemented. I have tried to suggest changes to the glossary to bring it closer to what has been implemented.

Comment thread docs/spec/overview.md Outdated

The primary resources in the Knative Eventing _Feeds_ API are EventSource, EventType and Bind:

* An **EventSource** wraps an event producer and is able to produce multiple definitions of
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.

EventSource.feeds.knative.dev vs Source.feeds.knative.dev

hmm... I do like that EventType matches, because these are tied together.

EventType.feeds.knative.dev

But type is a reserved word in golang.

Cloudevents uses eventType and source. so... I am in favor of changing to Source.

Comment thread docs/spec/overview.md Outdated

The primary resources in the Knative Eventing _Channels_ API are Channel, Subscription and Bus:

* A **Channel** is a logical endpoint to allow events to flow into a
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.

I think it would be more clear as real sentences too. Let me just rework it and not try to copy the style of serving.

Copy link
Copy Markdown
Member

@evankanderson evankanderson left a comment

Choose a reason for hiding this comment

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

At what point do you want to commit some part of this and leave the others with TODOs for a future PR?

We should also review this at an eventing WG at some point.

Comment thread docs/spec/motivation.md Outdated
# Motivation

Knative Eventing implements the common components for an event delivery
ecosystem. These common components enable producing and consuming events,
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.

I don't think you need the commas here, assuming you're attempting to say "(producing and consuming) (events adhering to the CloudEvents Specification) in a decoupled way."

Comment thread docs/spec/motivation.md Outdated
integrate with existing message consumers.

The Knative Eventing APIs is intended to operate independently, and interop
well with the [Compute API](https://github.com/knative/serving) and [Build
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.

Prefer "Serving API", since that's the package and repo name.

Comment thread docs/spec/motivation.md Outdated
primitives for event production and delivery with a focus on push over HTTP. If a
new event source or type is required of your application, the effort required
to plumb them into the existing eventing framework will be minimal and will
integrate with existing message consumers.
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.

I think it's also worth calling out middleware as well as consumers.

You might want to say "CloudEvents" rather than "existing", because CloudEvents is still quite young.

Comment thread docs/spec/overview.md Outdated
* **Subscription**, an expressed interest in events to be delivered to a
service.

![TODO:: Object model](images/object_model.png)
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.

This will be a broken image tag. Put it in comments?

Comment thread docs/spec/overview.md Outdated

* **EventType**, the schema for an event.

* **Bind**, ties the output of an event producer to the input of an event
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.

No comma here on Bind between subject and verb. If you want the same construction as above, try:

** Bind **, the association between the output ...

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think the EventSource in many cases is not actually the source of the event. If you look at the Pub/Sub EventSource, the topics will most likely not be the sources. In the case of CloudEvents, the source attribute will contain something different. Pub/Sub is then the infrastructure propagating the events of potentially many different sources.

Comment thread docs/spec/overview.md

# Eventing

TODO an overview of how these objects are created and perhaps the persona
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.

TAL at https://www.draw.io/#G1EgWHszjc8tO-5hW6_yoFBrWDt4F9TJcn / https://drive.google.com/open?id=1EgWHszjc8tO-5hW6_yoFBrWDt4F9TJcn if you want a sample picture of things working together.

I'm trying to find a good format to actually check that image into a repo. Sadly, UML does not have quite the layout control needed. I may end up doing SVG/PNG + the draw.io XML, which would make me slightly sad.

Comment thread docs/spec/motivation.md Outdated

Kubernetes has no primitives related to event processing, yet this is an
essential component in serverless workloads. Eventing introduces high-level
primitives for event production and delivery with a focus on push over HTTP. If a
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If we're going to emphasize HTTP here, perhaps we should qualify with a "currently". We hope to have gRPC support soon, and it might be the preferred approach for many use-cases since that would enable the use of streams.

Comment thread docs/spec/overview.md Outdated

## Channel

A named endpoint on a Bus which accepts events delivered from an outside system.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Currently Channels are only exposed as in-cluster Services. We have a PoC for a "gateway" (another naming conflict we'll have to resolve) that is exposed as an ingress, but in general, Channels may be used for app-to-app (or function-to-function) interacts within the cluster as well.

Comment thread docs/spec/overview.md Outdated

## Bus

A system which routes events from Channels to Actions. The Broker MAY (RFC
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would prefer "to subscribed Service endpoints" rather than "to Actions" here.

Comment thread docs/spec/overview.md

* **Subscription**, an expressed interest in events to be delivered to a
service.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This definition of Subscription might cause confusion in combination with Bind.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is another reason I prefer "Feed" since that would then emphasize its role as the manifestation of events from a source, and if it simply has a target (k8s) Service, that could be a direct connection to a Knative workload via the Service created by its Route, or it could be the Service that corresponds to a Channel. Only when using a Channel would there be something to "subscribe" to.

@google-prow-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: n3wscott
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: evankanderson

If they are not already assigned, you can assign the PR to them by writing /assign @evankanderson in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@mattmoor mattmoor assigned vaikas and unassigned mattmoor Jul 13, 2018
Copy link
Copy Markdown
Contributor

@grantr grantr left a comment

Choose a reason for hiding this comment

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

/lgtm
Thanks @n3wscott, would love to see this in the repo.

@markfisher @deissnerk I think your comments are resolved (hold if not). @evankanderson I'll let you approve when you're ready.

Comment thread docs/spec/overview.md

# Resource Types

The primary resources in the Knative Eventing _Feeds_ API are EventSource, EventType and Bind:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bind -> Feed

Comment thread docs/spec/motivation.md
to plumb them into the existing eventing framework will be minimal and will
integrate with CloudEvents middleware and message consumers.

The Knative Eventing APIs is intended to operate independently, and interop
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: APIs is should be changed to either API is or APIs are.

Is "intended to operate independently" an accurate description of the relationship between serving and eventing? My understanding is that the API definition and control plane are intended to be independent of serving, but the data plane (bus, channel, and feed implementations) have no such intent.

@google-prow-robot google-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants