-
Notifications
You must be signed in to change notification settings - Fork 10
Add support for schema_uri validation #87
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
ignore: | ||
- docs/examples/** |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,88 @@ | ||
package main | ||
|
||
import ( | ||
"context" | ||
"fmt" | ||
"log" | ||
"os" | ||
|
||
examples "github.com/cdevents/sdk-go/docs/examples" | ||
cdevents "github.com/cdevents/sdk-go/pkg/api" | ||
cdeventsv04 "github.com/cdevents/sdk-go/pkg/api/v04" | ||
cloudevents "github.com/cloudevents/sdk-go/v2" | ||
) | ||
|
||
const customSchemaUri = "https://myregistry.dev/schemas/cdevents/quota-exceeded/0_1_0" | ||
|
||
type Quota struct { | ||
User string `json:"user"` // The use the quota applies ot | ||
Limit string `json:"limit"` // The limit enforced by the quota e.g. 100Gb | ||
Current int `json:"current"` // The current % of the quota used e.g. 90% | ||
Threshold int `json:"threshold"` // The threshold for warning event e.g. 85% | ||
Level string `json:"level"` // INFO: <threshold, WARNING: >threshold, <quota, CRITICAL: >quota | ||
} | ||
|
||
func main() { | ||
var ce *cloudevents.Event | ||
var c cloudevents.Client | ||
|
||
// Define the event type | ||
eventType := cdevents.CDEventType{ | ||
Subject: "quota", | ||
Predicate: "exceeded", | ||
Version: "0.1.0", | ||
Custom: "myregistry", | ||
} | ||
|
||
// Define the content | ||
quotaRule123 := Quota{ | ||
User: "heavy_user", | ||
Limit: "50Tb", | ||
Current: 90, | ||
Threshold: 85, | ||
Level: "WARNING", | ||
} | ||
|
||
// Create the base event | ||
event, err := cdeventsv04.NewCustomTypeEvent() | ||
examples.PanicOnError(err, "could not create a cdevent") | ||
event.SetEventType(eventType) | ||
|
||
// Set the required context fields | ||
event.SetSubjectId("quotaRule123") | ||
event.SetSource("my/first/cdevent/program") | ||
|
||
// Set the required subject fields | ||
event.SetSubjectContent(quotaRule123) | ||
event.SetSchemaUri(customSchemaUri) | ||
|
||
// Print the event | ||
eventJson, err := cdevents.AsJsonString(event) | ||
examples.PanicOnError(err, "failed to marshal the CDEvent") | ||
fmt.Printf("%s", eventJson) | ||
|
||
// To validate the event, we need to load its custom schema | ||
customSchema, err := os.ReadFile("myregistry-quotaexceeded_schema.json") | ||
examples.PanicOnError(err, "cannot load schema file") | ||
|
||
err = cdevents.LoadJsonSchema(customSchemaUri, customSchema) | ||
examples.PanicOnError(err, "cannot load the custom schema file") | ||
|
||
ce, err = cdevents.AsCloudEvent(event) | ||
examples.PanicOnError(err, "failed to create cloudevent") | ||
|
||
// Set send options | ||
source, err := examples.CreateSmeeChannel() | ||
examples.PanicOnError(err, "failed to create a smee channel") | ||
ctx := cloudevents.ContextWithTarget(context.Background(), *source) | ||
ctx = cloudevents.WithEncodingBinary(ctx) | ||
|
||
c, err = cloudevents.NewClientHTTP() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why don't we wrap the cloudevent library to hide all this from the user? This example would then just be something like client, err := cdevents.NewHTTPClient()
if err != nil {
panic(err)
}
result, err := client.Send(event) This becomes much simpler I feel like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is on purpose - the CloudEvents API is rich, as it supports multiple transport options each with its own configuration capabilities. I don't think we should replicate that on CDEvents side, transport is not CDEvent's concern. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wrapping it does not mean a user cannot pass their own cloudevent client in. That can be done via optional params. This is standard in a lot of go sdks There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not convinced that: client, err := cdevents.NewClientCloudEventsHTTP() is going to provide a better user experience than: client, err := cloudevents.NewClientHTTP() The only difference is the import of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is where my experience in SDKs come in. If you wrap it, you have full control on what it looks like to the user. You can add hooks, etc. With your current implementation you cannot without fully understanding two SDKs. Two! That's a bad experience |
||
examples.PanicOnError(err, "failed to create the CloudEvents client") | ||
|
||
// Send the CloudEvent | ||
// c is a CloudEvent client | ||
if result := c.Send(ctx, *ce); cloudevents.IsUndelivered(result) { | ||
log.Fatalf("failed to send, %v", result) | ||
} | ||
} |
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 really should look into optional parameters at some point. Makes using things a little easier.
This adds some flexibility on where we can add validation as well, as in we could choose to do it upon creating the struct with a
cdevents.WithQuickValidate()
or something as an optionThere 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 was debating between explicit setters/getters and providing some kind of context object where users could stash fields to be set:
event.WithSubjectId("foo").WithSource("bar")
But that leads to as many
WithX
methods asSetX
methods to be defined, and I didn't really see an advantage. For the subject content, I introducedSetSubjectContent
which allows setting the whole content from a Struct directly rather than field by field.I'd like to better understand what you are proposing here, maybe something for the next WG.