-
Notifications
You must be signed in to change notification settings - Fork 43
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
First commit of extra examples #230
base: main
Are you sure you want to change the base?
Conversation
{ | ||
public class Startup : FunctionsStartup | ||
{ | ||
public override void ConfigureServices(WebHostBuilderContext context, IServiceCollection services) => |
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.
It is a bit odd to me that the formatter is configured as a Functions Framework service and not on the Google Events library. Said another way: how do we expect customers will use the Google Events library outside of the Functions Framework? For example consider the use case of deploying an ASP.NET app to CloudRun and point an EventArc subscription at it. I would expect to be able to do something like:
// using stuff
var builder = WebApplication.CreateBuilder(args);
// I guess would need to configure a service here?
var app = builder.Build();
app.MapPost("/", (CloudEvent cloudEvent) => {
// now I want to work with a Google Event data payload...
switch (cloudEvent.Data)
{
case StorageObjectData storage:
Console.WriteLine($"Storage Bucket: {storage.Bucket}");
break;
// etc...
}
});
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.
This is in a scenario where you're not using the Google Events library at all - it's not a Google event, it's a custom event payload.
Using the Google Events library outside the Functions Framework, you'd need to set up code to receive the CloudEvent - that's one area of the SDK that's still slightly TBD, but doesn't feel like it's in the remit of the Google Events library particularly - that should make it easy to parse the payload as a Google event payload, but that's all. (If you're writing a web server to handle a non-Google-payload CloudEvent request, it's not really our business, even if it happens that the request is made by Eventarc.)
switch (cloudEvent.Data) | ||
{ | ||
case StorageObjectData storage: | ||
Console.WriteLine($"Storage Bucket: {storage.Bucket}"); | ||
break; | ||
case MessagePublishedData pubsub: | ||
Console.WriteLine($"PubSub subscription: {pubsub.Subscription}"); | ||
break; | ||
default: | ||
Console.WriteLine($"Unhandled event type {cloudEvent.Type}"); | ||
break; | ||
} |
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.
😍
/// <summary> | ||
/// A CloudEvent formatter that knows about all the events in the Google.Events.Protobuf package. | ||
/// This would be in that package. | ||
/// </summary> | ||
public class GoogleEventFormatter : CloudEventFormatter | ||
{ | ||
private static Dictionary<string, MessageDescriptor> s_eventTypeToPayloadType = new Dictionary<string, MessageDescriptor> | ||
{ | ||
{ StorageObjectData.ArchivedCloudEventType, StorageObjectData.Descriptor }, | ||
{ StorageObjectData.DeletedCloudEventType, StorageObjectData.Descriptor }, | ||
{ StorageObjectData.FinalizedCloudEventType, StorageObjectData.Descriptor }, | ||
{ StorageObjectData.MetadataUpdatedCloudEventType, StorageObjectData.Descriptor }, | ||
|
||
{ MessagePublishedData.MessagePublishedCloudEventType, MessagePublishedData.Descriptor }, | ||
|
||
{ DocumentEventData.CreatedCloudEventType, DocumentEventData.Descriptor }, | ||
{ DocumentEventData.DeletedCloudEventType, DocumentEventData.Descriptor }, | ||
|
||
{ ReferenceEventData.CreatedCloudEventType, ReferenceEventData.Descriptor }, | ||
{ ReferenceEventData.DeletedCloudEventType, ReferenceEventData.Descriptor }, | ||
{ ReferenceEventData.WrittenCloudEventType, ReferenceEventData.Descriptor }, | ||
{ ReferenceEventData.UpdatedCloudEventType, ReferenceEventData.Descriptor }, | ||
|
||
// Etc... this would all be generated. | ||
}; |
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.
This is looks very close to what I had in mind for Typescript.
public override void ConfigureServices(WebHostBuilderContext context, IServiceCollection services) => | ||
services.AddSingleton<CloudEventFormatter, GoogleEventFormatter>(); |
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.
Open question: can we get to a point where this is the pre-configured default in the Functions Framework so developers don't need to provide a FunctionsStartup
to for this use case?
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.
See the readme for more thoughts on this. We could potentially have an IGoogleCloudEvent
marker interface that's implemented by all the concrete types, and then decorate that with the formatter reference. Customers would then write an ICloudEventFunction<IGoogleCloudEvent>
implementation.
Having this as a vanilla default is also easy if we're happy to a) make the Functions Framework depend on the Google Events library; b) favour Google event payloads over other payloads. Lots of options to talk about...
case MessagePublishedData pubsub: | ||
Console.WriteLine($"PubSub subscription: {pubsub.Subscription}"); | ||
break; | ||
default: |
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.
it is possible to combine this with the 3rd party sample somehow? So I can have a case
like:
case MyServiceSpecificPayload payload:
Console.WriteLine("This isn't a Google Event but it is a known event");
break;
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.
I think that gets tricky - somewhere you've got to know where to get all the event types from. But we can think about it...
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.
I think this is probably less common / lower priority use case so I don't think we need to go out of our way to optimize for it.
This PR is to investigate possible use cases, and how easily they could be achieved.