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

add csharp_namespace option #227

Open
Varorbc opened this issue Jun 4, 2024 · 11 comments
Open

add csharp_namespace option #227

Varorbc opened this issue Jun 4, 2024 · 11 comments

Comments

@Varorbc
Copy link

Varorbc commented Jun 4, 2024

No description provided.

@Varorbc
Copy link
Author

Varorbc commented Jun 4, 2024

@bestbeforetoday Can I create a pr?

@jt-nti
Copy link
Member

jt-nti commented Jun 4, 2024

@Varorbc historically the fabric protos have included language specific options like go_package, which isn't ideal. The language bindings now get generated using Buf, which has a managed mode to take care of those options outside the proto files, which works really well.

Buf managed mode seems to have C# support -> https://buf.build/docs/generate/managed-mode#c#

(It would be really interesting to get a PR to add C# to the three current language bindings that get generated.)

@Varorbc
Copy link
Author

Varorbc commented Jun 4, 2024

@jt-nti Thank you for providing another solution. But for users who use C#, the function of generating code according to proto has been provided, and we may prefer to choose this way instead of introducing external dependencies.It would be great if you could add the namespace option to the proto file.

https://learn.microsoft.com/aspnet/core/grpc/basics

@jt-nti
Copy link
Member

jt-nti commented Jun 4, 2024

@Varorbc I wouldn't object to csharp_namespace options being added to the proto files but experience with the Go, Java and Node bindings is that it is much easier for API consumers to just use a pre-generated module instead of having to generate anything themselves.

I don't know C# but I assume that it would be possible to do the same kind of thing, in which case building on the existing Buf framework in this repo to generate and publish C# bindings would be ideal.

@jt-nti
Copy link
Member

jt-nti commented Jun 6, 2024

I've been experimenting with adding C# bindings in #230 but I'm not really sure what's required to make it a useful/publishable package. Any ideas welcome!

@Varorbc
Copy link
Author

Varorbc commented Jun 6, 2024

I have done message binding and implemented the gateway client with reference to Java. After you release the preview package, I will replace it.

@bestbeforetoday
Copy link
Member

@Varorbc I notice you closed your two pull requests to add csharp namespace options to the protobuf files. Did you find a different approach to make things work for your csharp client API implementation? It would be great to share your implementation with the community. If it's complete enough for others to use, it might even be something you want to contribute as a new Hyperledger lab or project.

@Varorbc
Copy link
Author

Varorbc commented Sep 18, 2024

@bestbeforetoday Sorry, but this has been dragging on for too long, so I temporarily copied protobuf into my project and added the C# namespace options. Right now, our client is working well with Hyperledger Fabric. As for the experimental project you mentioned, I need to get approval from my boss since I don’t have the authority to decide on this.

@bestbeforetoday
Copy link
Member

Generating the protobuf bindings within your own project is definitely a viable approach. This used to happen with most of the official Fabric client SDKs too. It's only when we released the Fabric Gateway client API alongside Fabric v2.4 that we created the current artifacts for Node and Java, and the Go bindings based on the Go protobuf APIv2.

@jt-nti
Copy link
Member

jt-nti commented Sep 18, 2024

@bestbeforetoday generating protobuf bindings yourself is definitely viable but it would be nice to at least get the namespaces in the official protobuf definitions. I think @Varorbc was pretty close to having that done (I would have liked to see a minor change #228 (comment) but that's not a huge issue) unfortunately none of the project maintainers reviewed the PRs.

I've not had time to do any more investigation on publishing pre-generated bindings but I would be happy to create new PRs to get the namespaces in if anyone is willing to approve them.

@Varorbc
Copy link
Author

Varorbc commented Sep 18, 2024

@bestbeforetoday If multiple projects need protobuf, then generating a unified binding for protobuf makes sense. But if it’s just one project, setting it up directly in that project should be fine. Also, I talked to my boss about contributing to the Fabric Gateway C# client API as a Hyperledger experimental project, and he said he’ll consider it.

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

3 participants