protobuf icon indicating copy to clipboard operation
protobuf copied to clipboard

No way to use custom options in Ruby

Open kailan opened this issue 8 years ago • 35 comments

I created a custom option extension that I use in my proto files, like this:

service Fishtank {
  rpc GetFish(FishQuery) returns (Fish) {
    option (protapi.http).get = "/fish/{id}";
  }
}

It doesn't seem that once it is compiled into Ruby code, I can access the value of the protapi.http option. I believe this can be done in other languages by accessing the descriptor.

Am I missing something, or is this not possible right now?

kailan avatar Jan 29 '16 20:01 kailan

You are correct that Ruby does not yet support custom options. Hopefully we can add this before too long.

haberman avatar Jan 30 '16 00:01 haberman

@haberman Awesome! It'll really help out a project of mine.

Any way I can help out with implementing this?

kailan avatar Jan 30 '16 00:01 kailan

This would be helpful for me as well

skipperguy12 avatar Jan 31 '16 20:01 skipperguy12

This is becoming a more and more common request. I have been thinking through how the implementation ought to work on this. It's slightly tricky to modify the DSL for this, because custom options let you declare the option and use it both in the same file. This is a challenge to accommodate in a clean way.

haberman avatar Feb 10 '16 21:02 haberman

Any progress on this? @haberman

kailan avatar Feb 28 '16 01:02 kailan

Ok there are a couple parts to making this work.

First of all we need to support proto2 field presence. Right now the code generator fails immediately if it sees a file with syntax="proto2";. Step 1 would be a PR that supports proto2 files that don't contain any extensions or custom options. Proto2 "optional" fields should be supported by adding hasbits to the in-memory representation to indicate whether a field is present or not. I'm happy to explain this in more detail for anyone who wants to implement this.

Secondly we need to support proto2 extensions. This means extending the DSL with something like add_extension and making it so you can read and write extension fields of extended messages with something like msg[my_extension] = 5.

Finally we need a way of extending the DSL to support custom options. We can talk more about this part once #1 and #2 are done.

I'd be happy to see pull requests for any/all of these, and I'd be happy to help guide people who want to implement this to the right places in the code.

haberman avatar Feb 29 '16 21:02 haberman

I'd love to work on this but I have one question: Why would proto2 need to be supported? Surely implementing this for proto3 would be enough.

kailan avatar Mar 01 '16 19:03 kailan

Custom options are defined as extensions of messages descriptor.proto, which is a syntax="proto2"; file. Extensions don't exist at all in proto3.

So even if your messages are only proto3, we need proto2 support to be able to represent the options themselves.

haberman avatar Mar 01 '16 19:03 haberman

Ah, makes sense. I'll give it a shot later on :+1:

kailan avatar Mar 01 '16 20:03 kailan

Any status updates on proto2 support?

nerdrew avatar Jan 26 '17 21:01 nerdrew

Any progress since last year? I might have a few days to push this forward.

djudd-stripe avatar Aug 28 '17 01:08 djudd-stripe

Funny. We were thinking of looking at this soon too. cc @zanker

nerdrew avatar Aug 28 '17 03:08 nerdrew

Hah, @djudd-stripe I was actually talking to @nerdrew about this last week and we assumed that Stripe didn't use protobufs. Are you using the "protobuf" gem currently?

I was able to hack something to support defaults in a few days, so I don't think it's too hard to at least get basic protobuf2 support at least. It looks like most of the work is building out the Ruby <-> C bridges, as UPB handles everything already.

I'm still working on proving this out internally to make sure we can feasibly swap to this, but right now we're interested in adding at least basic synta2 support.

@haberman it sounds like on your end, you would be interested in merging syntax2 support to the Ruby gem, as long as it meets code standards and is done properly?

zanker avatar Aug 28 '17 05:08 zanker

Not a lot of Protobuf at Stripe right now but we are exploring adding more. We're using this code so far but are interested in custom annotations.

djudd-stripe avatar Aug 28 '17 17:08 djudd-stripe

@zanker it sounds like you have Proto2 support work in-progress? Anything more useful I can do than wait for you to get it merged, then maybe add custom option support on top (unless you've included that already)?

djudd-stripe avatar Aug 28 '17 18:08 djudd-stripe

Thanks for the offer, I don't think there's anything right now. I was able to get a more proper one (using hasbits) working, I need a few days to sort it out and clean/test things and I'll submit a PR.

I'm just starting with the syntax2/default parts initially. If you wanted to add custom option support on top of that then 👍 . Will end up doing it otherwise.

zanker avatar Aug 28 '17 22:08 zanker

@djudd-stripe This is still very much WIP (hence the random debug statements), but for reference: https://github.com/google/protobuf/compare/master...zanker:zanker/syntax2 is what things are looking like. I don't think wiring in extensions would be too hard overall.

zanker avatar Aug 30 '17 15:08 zanker

Any progress on this? AFAIK there is no way to work with descriptor.proto in ruby.

syntax = "proto3";
package foo;
import "google/protobuf/descriptor.proto";

:cry:

pcj avatar Feb 01 '18 01:02 pcj

Sorry! I left Square back in October and the team who was going to take it over got reshuffled so they're not going to have time to pick it back up.

The PR is at https://github.com/google/protobuf/pull/3606, and all the proto side code is pretty solid from the testing I did. The blocker was the proto syntax, which wasn't narrowed down until after I had already left.

If you want to pick up that part of the PR, I'd be happy to walk you through on whatever part of the PR isn't clear.

zanker avatar Feb 01 '18 04:02 zanker

Is where any plans to implement custom options in ruby? Custom options looks great for expressions some meta logic and it is so sad what ruby does not include this abilitites.

My primary interest is extending enum options.

pechorin avatar Nov 18 '19 15:11 pechorin

It looks like there is support in upb.c. What is the thinking around what the syntax would look like? If it's straightforward I can put up a PR.

ebenoist avatar Mar 17 '20 16:03 ebenoist

Custom options currently cannot be compiled via protoc2, but can be compiled via protoc3 but options will be skipped in compiled ruby pb file. So it will be cool to provide meta options like this in ruby output file:

syntax = "proto2";

import "google/protobuf/descriptor.proto";

extend google.protobuf.EnumValueOptions {
   optional string name = 1001;
}
message Operation {

  enum ClassName {
    OPERATION = 0 [(name) = 'Operation'];
    PUSH_OPERATION = 1  [(name) = 'PushOperation'];
  }
}

So, you ask about how it will be implemented in ruby generated classes? Am i understand you right?

pechorin avatar Mar 17 '20 18:03 pechorin

@pechorin Thats correct, just looking for guidance on the Ruby syntax/methods for the generated classes. It seems like upb.c already has support for reading out the options, they would just need to be exposed.

ebenoist avatar Mar 18 '20 14:03 ebenoist

Any news on this? I also wanted to add options to an enum and it works perfectly for go but not ruby 😞

syntax = "proto3";

package some.package;

import "google/protobuf/descriptor.proto";

extend google.protobuf.EnumOptions {
  optional int64 default_http_status_code = 500;
}

extend google.protobuf.EnumValueOptions {
  optional int64 http_status_code = 501;
}

message Error {
  enum Code {
      option (default_http_status_code) = 400;

      CODE_UNSPECIFIED = 0 [(http_status_code) = 500];
      CODE_INVALID = 1;
      CODE_EXPIRED = 2;
      CODE_NOT_FOUND = 3 [(http_status_code) = 404];
  }

  Code code = 1;
}
[libprotobuf WARNING ../../third_party/protobuf/src/google/protobuf/compiler/ruby/ruby_generator.cc:512] Omitting proto2 dependency 'google/protobuf/descriptor.proto' from proto3 output file 'banked/consent/v1/errors_pb.rb' because we don't support proto2 and no proto2 types from that file are being used.

krak3n avatar Jun 22 '21 11:06 krak3n

Hello, can we please get an update on plans for this? We, too, have a heterogenous stack with Go, Python, and Ruby and are blocked from using this feature because of this issue.

// metadata3.proto
syntax = "proto3";

import "google/protobuf/descriptor.proto";

package metas3;

message Metadata3 {
  optional string field1 = 1;
  optional string field2 = 2;
  optional string field3 = 3;
}

extend google.protobuf.MessageOptions {
  Metadata3 metadata3 = 50000;
}
// event.proto
syntax = "proto3";

import "metadata3.proto";

package events;

message Event {
  string id = 1;
  string name = 2;
  string someData = 3;
  option (metas3.metadata3).field1 = "placeholder1";
  option (metas3.metadata3).field2 = "placeholder2";
}
// No extensions in generated code
# Generated by the protocol buffer compiler.  DO NOT EDIT!
# source: metadata3.proto

require 'google/protobuf/descriptor_pb'
require 'google/protobuf'

Google::Protobuf::DescriptorPool.generated_pool.build do
  add_file("metadata3.proto", :syntax => :proto3) do
    add_message "metas3.Metadata3" do
      proto3_optional :field1, :string, 1
      proto3_optional :field2, :string, 2
      proto3_optional :field3, :string, 3
    end
  end
end

module Metas3
  Metadata3 = ::Google::Protobuf::DescriptorPool.generated_pool.lookup("metas3.Metadata3").msgclass
end
require_relative 'protos_generated/event_pb'

include Events

class MetadataTest
  def to_event
    Event.new(
      id: '1',
      name: 'name',
      someData: 'someData'
    )
  end

  def verify
    to_event = self.to_event
    binary_encoding = Event.encode(to_event)
    binary_decoding = Event.decode(binary_encoding)
    raise "Binary encoding error" unless binary_decoding == to_event
  end
end

Running SerDe:

bundle exec ruby -r "./metadata_test.rb" -e "MatadataTest.new.verify"

Traceback (most recent call last):
...
kernel_require.rb:54:in `require': cannot load such file -- google/protobuf/descriptor_pb (LoadError)
...
google-protobuf-3.18.1-universal-darwin/lib/google/protobuf/descriptor_pb.rb:5:in `<top (required)>': uninitialized constant Google (NameError)

Edit: What confuses me, though, is that this error doesn't exist in bundle console!

bundle console
2.5.5 :001 > load 'metadata_test.rb'
 => true
2.5.5 :002 > MetadataTest.new.verify
 => nil
2.5.5 :003 >

i0jupiter avatar Oct 12 '21 18:10 i0jupiter

Also, is there any other mechanism in proto3 that we can use to get similar functionality for the time being?

i0jupiter avatar Oct 12 '21 18:10 i0jupiter

So I have been playing with this more and found that the code generated by protobuf/releases/download/v3.17.3/protoc-3.17.3-linux-x86_64.zip is different from protobuf/releases/download/v3.17.3/protoc-3.17.3-osx-x86_64.zip!

The linux one doesn't have the require 'google/protobuf/descriptor_pb' line, so the above error doesn't exist. Tracking this down has been an incredibly frustrating experience—I hope you choose to do something about it.

This also means that, while extensions are still unavailable in Ruby, at least proto definitions with extensions can be used with Ruby in a multi-lang stack.

i0jupiter avatar Oct 17 '21 01:10 i0jupiter

I'm sorry this has been frustrating to track down.

I was not able to reproduce your report of getting different output from protoc-3.17.3-linux-x86_64.zip and protoc-3.17.3-osx-x86_64.zip. I tried them both just now and did not get the require 'google/protobuf/descriptor_pb' line in either.

If you are getting require 'google/protobuf/descriptor_pb' I think you must be using a build of protoc that includes https://github.com/protocolbuffers/protobuf/pull/9003, which was released in https://github.com/protocolbuffers/protobuf/releases/tag/v3.18.1.

I think the root cause of the issue you encountered is a bug in https://github.com/protocolbuffers/protobuf/pull/9003. Specifically, the generated file descriptor_pb.rbis throwing an error. This error occurs because google/protobuf has not yet been required:

google-protobuf-3.18.1-universal-darwin/lib/google/protobuf/descriptor_pb.rb:5:in `<top (required)>': uninitialized constant Google (NameError)

We need to change the code generator such that descriptor_pb.rb starts with the line require 'google/protobuf'. A short-term workaround would be to manually require 'google/protobuf' before importing any other foo_pb generated file.

All of this is unrelated to whether Ruby supports custom options or not (just noting this for future readers who come across this bug).

Custom options for Ruby are now within reach; the upb library which powers Ruby supports extensions as of https://github.com/protocolbuffers/upb/pull/421 and custom options as of https://github.com/protocolbuffers/upb/pull/426. Full custom option support for Ruby should be possible now.

haberman avatar Oct 17 '21 02:10 haberman

Hello @haberman, want to confirm that I was using 3.17.3 linux but 3.18.1 osx, so the bug from #9003 makes sense (and is unfortunate). 3.17.3 is what our official schema repo is using, but I'd recently upgraded my personal setup to the latest protoc. Thank you for pointing this out!

I know that this is a public forum, but would you be comfortable predicting when extensions might work in Ruby? :) Also happy to chat offline.

I have another question about how extensions themselves work. Why do we have to provide a placeholder value for every option in the message using options (example)? This beats the benefit of encapsulating all the custom options in a single message. If the options message adds a new field, we'd need to update every message using the options before they can use it. This is very counterintuitive. Would love to know the design principle behind this.

i0jupiter avatar Oct 19 '21 00:10 i0jupiter

I opened a separate issue for the problem with google/protobuf/descriptor_pb: https://github.com/protocolbuffers/protobuf/issues/9122

This will be fixed in https://github.com/protocolbuffers/protobuf/pull/9121

I misspoke; this was not a bug in https://github.com/protocolbuffers/protobuf/pull/9003, this was a bug in my previous PR https://github.com/protocolbuffers/protobuf/pull/8850

@i0jupiter I'll address your other questions momentarily.

haberman avatar Oct 19 '21 22:10 haberman