-
Notifications
You must be signed in to change notification settings - Fork 15.4k
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
MethodDescriptor.CustomOptions no longer works in 3.11.0-rc2 (when old generated code is used) #6956
Comments
In the new API, extension identifiers are generated automatically in container classes like using Google.Api;
var extension = AnnotationsExtensions.Http;
var rule = methodDescriptor.GetOption(extension); The issue with the extension not being discovered is due to an issue I believe in the |
Actually, wait. Are you loading descriptors dynamically in this case @JamesNK? |
I don't know what it means to get descriptors dynamically or not. I have this proto file: syntax = "proto3";
import "google/api/annotations.proto";
package greet;
// The greeting service definition.
service Greeter {
// Sends a greeting
rpc SayHello (HelloRequest) returns (HelloReply) {
option (google.api.http) = {
get: "v1/greeter/{name}"
};
}
}
// The request message containing the user's name.
message HelloRequest {
string name = 1;
string test_name = 2;
}
// The response message containing the greetings
message HelloReply {
string message = 1;
} I reference it in my csproj with I have a reference Google.Api.CommonProtos, required for generated by descriptor = pbr::FileDescriptor.FromGeneratedCode(descriptorData,
new pbr::FileDescriptor[] { global::Google.Api.AnnotationsReflection.Descriptor, },
new pbr::GeneratedClrTypeInfo(null, new pbr::GeneratedClrTypeInfo[] {
new pbr::GeneratedClrTypeInfo(typeof(global::Greet.HelloRequest), global::Greet.HelloRequest.Parser, new[]{ "Name", "TestName" }, null, null, null),
new pbr::GeneratedClrTypeInfo(typeof(global::Greet.HelloReply), global::Greet.HelloReply.Parser, new[]{ "Message" }, null, null, null)
})); And then I am getting the |
using Google.Api;
var extension = AnnotationsExtensions.Http;
var rule = methodDescriptor.GetOption(extension); I don't have |
Oh I see. In this case you're mixing generated code from different versions. The code from gax-dotnet is using another previous version of code-gen while you're using a future version Google.Protobuf that's expecting you to have extension identifiers generated. |
I keep forgetting, is this kind of setup supported @jtattermusch? |
Would a solution be to generate code from |
Yes that'd work. All you need is the |
Hmm, but it won't work because And making the types public could easy create conflicts with anyone who is referencing Google.Api.CommonProtos. You either need to do:
Right now I don't see a way for this to work. |
When will the next patch release be? |
No idea, you'd have to ask @rafi-kamal. |
We are going to do a new release as soon as all the RC2 issues are fixed. I think this one and #6936 are the only one that need to be fixed. |
What's the state of this issue? I tested the latest package - 3.11.2 - and it is still broken. |
Looks like the fix didn't make into the final release. Unfortunately we don't have any ETA for the next release at this moment. |
All I need to finish #7005 up is write some sort of tests for it. I'm going to write a quick and easy version using a slightly modified version of |
#6936 isn't released yet either. (Just taking advantage of it being mentioned in this issue earlier) |
Would be any release soon to fix this issue? |
@JamesNK that's why https://github.com/aspnet/AspLabs/tree/master/src/GrpcHttpApi doesn't work now? Edited: |
HI all. I have been watching this issue for the past few months as we are rely on the custom options. Is there any update? |
Actually, I think CustomOptions would just work fine for you if you use the latest version or both the generated code and the runtime - AFAIK the problem in this issue was that CustomOptions didn't work if you had a new protobuf runtime, but an old version of the generated code. (The unmerged PR #7005 actually attempts to fix only the scenario where you have old generated code an a new runtime - everything else should work). Also, you can use the extensions API instead of relying on CustomOptions. |
I just closed the long standing PR #7005 which was an attempt to fix this.
|
Disregard the message below. RTFM. I missed the part about recompiling the proto files. Works great now! enum WorldRegionCode {
UNDEFINED_REGION = 0 [(protocommon.enum_pretty_name) = "{undefined}"];
NORTH_AMERICA = 1 [(protocommon.enum_pretty_name) = "North Americ"];
LATIN_AMERICA = 2 [(protocommon.enum_pretty_name) = "Latin America"];
EUROPE = 3 [(protocommon.enum_pretty_name) = "Europe"];
ASIA = 4 [(protocommon.enum_pretty_name) = "Asia"];
AUSTRALIA = 5 [(protocommon.enum_pretty_name) = "Australia"];
} Where the extend google.protobuf.EnumValueOptions {
string enum_pretty_name = 50002;
} Prior to the changes here, we could do this: //Get the enum descriptor by type
var enumDescriptor = MyProtoReflection.Descriptor.EnumTypes.FirstOrDefault(eDesc => eDesc.ClrType == typeof(TEnum));
var dict = new Dictionary<TEnum, string>();
//Need to extract the attributes in the most convoluted way imaginable
foreach (var edv in enumDescriptor.Values)
{
//See if there is a custom value;
var customValue = edv.CustomOptions.TryGetString(EnumPrettyNameField, out var value)
? value
: default;
var enumValue = (TEnum)Enum.ToObject(typeof(TEnum), edv.Number);
dict[enumValue] = customValue;
} but even in the latest version of the library we do not see it anywhere in the compiled Thanks. |
With new generated code, there should be an extension identifier named var customValue = edv.GetOptions()?.GetExtension(ProtocommonExtensions.EnumPrettyName) ?? default; |
@ObsidianMinor Thanks! Thats a much cleaner way to do it. Appreciate the help! Ernie |
I want to get custom options off a
MethodDescriptor
. Specifically I want to getHttpRule
which is from Google.Api.CommonProtos 1.7.0. In Google.Protobuf 3.8.0 I can successfully get this option by doing this:With Google.Protobuf 3.9.0 and 3.10.0
CustomOptions.TryGetMessage<T>
errors, I believe related to this issue: #6824When I updated to 3.11.0-rc2
CustomOptions.TryGetMessage<T>
no longer errors, but it also doesn't return a value. You should fix that for backwards compatibility.I see that CustomOptions has been made obsolete. How do I use the new API?
The text was updated successfully, but these errors were encountered: