Skip to content
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.

Sign up for GitHub

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

Create webhook-related components. #17

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Create webhook-related components. #17

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e049aad
Create webhook-related components.
rofrankel Aug 17, 2024
498a46d
lint
rofrankel Aug 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 65 additions & 0 deletions proto/aep-api/aep/api/webhook.proto
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

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

I think you also need to add something to schemas/? and json_schema too ideally.

Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
syntax = "proto3";

package aep.api;

import "google/protobuf/any.proto";
import "google/protobuf/descriptor.proto";
import "google/protobuf/timestamp.proto";

option java_package = "dev.aep.api";
option java_multiple_files = true;
option java_outer_classname = "NotificationProto";
option go_package = "aep.dev/api";

// Provides a generic wrapper for all webhook notifications.
message Notification {
Copy link
Member

Choose a reason for hiding this comment

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

thoughts on making this compatible with https://github.com/standard-webhooks/standard-webhooks/blob/main/spec/standard-webhooks.md?

// An ID that represents a single notification.
//
// The value of this field should be consistent across multiple attempts to
// send the same notification; clients can use it to deduplicate redundant
// notifications.
//
// This is typically, though not always, a UUID.
string id = 1;

// The type of the event that triggered the notification.
//
// The value of this field should directly correspond to the type of the
// payload.
//
// Clients can use this field to route the event without having to inspect the
// contents of the payload.
string event_type = 2;

// The time when the event that triggered the notification occurred.
google.protobuf.Timestamp event_time = 3;

// The payload of the notification.
//
// The type of this payload should directly correspond to the event type.
google.protobuf.Any payload = 4;
Copy link
Member

Choose a reason for hiding this comment

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

doesn't this have the same problem as any other .Any where you don't know the schema? and therefore you need some sort of side-documentation that isn't statically verified like JsonSchema?

seems like a pattern for defining webhook schema would be better than an actual type.

}

extend google.protobuf.MessageOptions {
// Indicates that the message is a webhook payload.
WebhookPayloadConfiguration webhook_payload = 50000;
}

// Contains metadata about webhook payloads.
//
// Example:
//
// ```proto
// // This notification is sent when a GroupJoinRequest is created; that is,
// // when a user requests to join a group.
// message BookArchived {
// option (aep.api.webhook_payload) = {
Copy link
Member

Choose a reason for hiding this comment

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

OOC - how are webhooks supposed to work in gRPC?

I feel like protobufs will only work when your server defines and RPC schema that the client knows how to send requests to - theoretically I guess the gRPC protocol does send over HTTP/2 so you as long as your message is packed via protobuf and the request payload matches someone could send a payload to you.

I suppose that's also the motivation for using protobuf.Any? you can send any payload type you want to and leave it to the webhook received to cast the object appropriately?

// event_type: "book_archived"
// };
// }
// ```
message WebhookPayloadConfiguration {
// The event type of the webhook notification that this payload is associated
// with.
string event_type = 1;
}
Loading