Skip to content

Conversation

@eamonnmcmanus
Copy link
Member

This allows received Legacy Events to be handled by function code that expects a
CloudEvent input. The translation logic and tests are based closely on the
corresponding code in https://github.com/GoogleCloudPlatform/functions-framework-dotnet.

The new code also passes the relevant conformance tests in
https://github.com/GoogleCloudPlatform/functions-framework-conformance, once
GoogleCloudPlatform/functions-framework-conformance#30
is applied. For now I'm running those tests manually. I plan at least to commit
the script and test functions that allow me to do so.

This means functions that receive a CloudEvent payload and dispatch it to a user function that expects a CloudEvent object. We do not yet handle converting a CloudEvent payload to the form expected by legacy GCF functions.
This allows received Legacy Events to be handled by function code that expects a
CloudEvent input. The translation logic and tests are based closely on the
corresponding code in https://github.com/GoogleCloudPlatform/functions-framework-dotnet.

The new code also passes the relevant conformance tests in
https://github.com/GoogleCloudPlatform/functions-framework-conformance, once
GoogleCloudPlatform/functions-framework-conformance#30
is applied. For now I'm running those tests manually. I plan at least to commit
the script and test functions that allow me to do so.
@eamonnmcmanus
Copy link
Member Author

My main concern here is about the maybeReshapeData method, which I added based on the dotnet original. I had to deduce what the resultant CloudEvent looks like, since I didn't see anything in the dotnet repo that tested it. But I might not have been looking hard enough.

@jskeet
Copy link
Member

jskeet commented Sep 11, 2020

My main concern here is about the maybeReshapeData method, which I added based on the dotnet original. I had to deduce what the resultant CloudEvent looks like, since I didn't see anything in the dotnet repo that tested it. But I might not have been looking hard enough.

Most of the testing is in https://github.com/GoogleCloudPlatform/functions-framework-dotnet/blob/master/src/Google.Cloud.Functions.Framework.Tests/GcfEvents/EventDeserializationTest.cs (and the test files in that directory)

But for the actual shape of the CloudEvent, is your friend.

Copy link
Member

@jskeet jskeet left a comment

Choose a reason for hiding this comment

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

Looks okay to me, although there may well be subtleties that I've missed. I've added a comment for a simplification you can probably make.

There will be more Firestore/Firebase work to do in terms of source and subject - I'm hoping to get the proposed CloudEvent format finalized soon. (Will include you in the doc.)

@eamonnmcmanus
Copy link
Member Author

Most of the testing is in https://github.com/GoogleCloudPlatform/functions-framework-dotnet/blob/master/src/Google.Cloud.Functions.Framework.Tests/GcfEvents/EventDeserializationTest.cs (and the test files in that directory)

Oh, excellent! It looks as if I can get better test coverage just by translating that test to Java, which I'll do in a later change.

But for the actual shape of the CloudEvent, is your friend.

Is there a word missing here?

@jskeet
Copy link
Member

jskeet commented Sep 11, 2020

Whoops - I could have sworn I copied the link. Doh. https://github.com/googleapis/google-cloudevents is the source of truth for the format of the data part within Google CloudEvents. (You may wish to consume that via https://github.com/googleapis/google-cloudevents-java when the code is published appropriately, in order to test the conversion.)

@eamonnmcmanus eamonnmcmanus merged commit fe091a5 into master Sep 11, 2020
@eamonnmcmanus eamonnmcmanus deleted the legacyeventtocloudevent branch September 11, 2020 15:27
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.

2 participants