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.

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

Unexpected auto-generated class name from CloudNative.CloudEvents.Protobuf #256

Closed
gbrulotte opened this issue Mar 3, 2023 · 4 comments
Closed

Comments

@gbrulotte
Copy link

Hi, we have a use case where we want to generate a CloudEvent in a service using CloudNative.CloudEvents and send it in the protobuf format using CloudNative.CloudEvents.Protobuf in a gRPC call. We defined a custom .proto containing the service and wanted to re-use the existing CloudEvent proto (https://github.com/cloudevents/spec/blob/main/cloudevents/formats/cloudevents.proto) within our service proto to be able to deserialize the event on the server side in a CloudNative.CloudEvents CloudEvent.

Since the CloudNative nuget package already bundles the auto-generated classes, we need to reference the CloudEvent proto without compiling it. With this configuration, I get this error:

error CS0234: The type or namespace name 'CloudeventsReflection' does not exist in the namespace 'CloudNative.CloudEvents.V1' (are you missing an assembly reference?)

From what I see in https://github.com/cloudevents/sdk-csharp/blob/main/generate_protos.sh, the cloudevents.proto is copied in a file named ProtoSchema.proto. This results in an auto-generated class named ProtoSchemaReflection (https://github.com/cloudevents/sdk-csharp/blob/main/src/CloudNative.CloudEvents.Protobuf/ProtoSchema.g.cs#L15) instead of being named CloudeventsReflection.

Would it be possible to instead keep the original proto file name (cloudevents.proto) to keep compatibility?

Here is a sample showing the problem: https://github.com/gbrulotte/CloudEventProtobufSample.

Thanks!

@jskeet
Copy link
Contributor

jskeet commented Mar 3, 2023

Unfortunately changing that now would be a breaking change for anyone who already refers to ProtoSchemaReflection. I suspect there was some problem with keeping the original name - I don't know why else I'd have changed it, to be honest.

I'll have a closer look on Monday.

@jskeet
Copy link
Contributor

jskeet commented Mar 6, 2023

Will try to look fully later on today, but one option I'll look into is renaming, but adding a handwritten ProtoSchemaReflection class which just delegates to CloudEventReflection, for compatibility.

jskeet added a commit to jskeet/sdk-csharp that referenced this issue Mar 6, 2023
For some reason we were renaming cloudevents.proto as ProtoSchema.proto, which breaks compatibility with anyone using the original file.

Renaming the reflection class is a breaking change, so I've added an obsolete compatibility class to keep compatibility.
Also updated to Google.Protobuf 3.22.0 (of the general protobuf 22.0 release; protobuf versioning is complex).

Fixes cloudevents#256
jskeet added a commit to jskeet/sdk-csharp that referenced this issue Mar 6, 2023
For some reason we were renaming cloudevents.proto as ProtoSchema.proto, which breaks compatibility with anyone using the original file.

Renaming the reflection class is a breaking change, so I've added an obsolete compatibility class to keep compatibility.
Also updated to Google.Protobuf 3.22.0 (of the general protobuf 22.0 release; protobuf versioning is complex).

Fixes cloudevents#256

Signed-off-by: Jon Skeet <[email protected]>
@jskeet jskeet closed this as completed in b188ee8 Mar 6, 2023
@jskeet
Copy link
Contributor

jskeet commented Mar 6, 2023

Will do a release tomorrow.

jskeet added a commit to jskeet/sdk-csharp that referenced this issue Mar 7, 2023
Changes since 2.5.1:

- Dependencies: system-level dependencies updated
- The NuGet package now uses PackageLicenseExpression (but still
  includes the licence file as well).
  Fixes ([cloudevents#252](cloudevents#252)).
- Regenerated protobuf schema using the original filename of
  cloudevents.proto instead of ProtoSchema.proto. An additional
  ProtoSchemaReflection class has been added purely for compatibility.
  Fixes ([cloudevents#256](cloudevents#256)).

Signed-off-by: Jon Skeet <[email protected]>
@jskeet jskeet mentioned this issue Mar 7, 2023
jskeet added a commit that referenced this issue Mar 7, 2023
Changes since 2.5.1:

- Dependencies: system-level dependencies updated
- The NuGet package now uses PackageLicenseExpression (but still
  includes the licence file as well).
  Fixes ([#252](#252)).
- Regenerated protobuf schema using the original filename of
  cloudevents.proto instead of ProtoSchema.proto. An additional
  ProtoSchemaReflection class has been added purely for compatibility.
  Fixes ([#256](#256)).

Signed-off-by: Jon Skeet <[email protected]>
@jskeet
Copy link
Contributor

jskeet commented Mar 7, 2023

Release 2.6.0 is now available - do let me know if you still have issues. (I've tried your sample project, and that now builds, so hopefully that was all that was needed.)

ericdotnet added a commit to ericdotnet/CSharp-sdk-dev that referenced this issue May 13, 2024
Changes since 2.5.1:

- Dependencies: system-level dependencies updated
- The NuGet package now uses PackageLicenseExpression (but still
  includes the licence file as well).
  Fixes ([#252](cloudevents/sdk-csharp#252)).
- Regenerated protobuf schema using the original filename of
  cloudevents.proto instead of ProtoSchema.proto. An additional
  ProtoSchemaReflection class has been added purely for compatibility.
  Fixes ([#256](cloudevents/sdk-csharp#256)).

Signed-off-by: Jon Skeet <[email protected]>
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

No branches or pull requests

2 participants