Avro
  1. Avro
  2. AVRO-495

Avro IDL should support includes

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.3.1
    • Fix Version/s: 1.4.0
    • Component/s: java
    • Labels:
      None

      Description

      includes would be a nice feature for Avro IDL

      1. AVRO-495.patch
        9 kB
        Doug Cutting
      2. AVRO-495.patch
        8 kB
        Doug Cutting
      3. AVRO-495.patch
        7 kB
        Doug Cutting

        Issue Links

          Activity

          Hide
          Doug Cutting added a comment -

          A simple include mechanism might just handle paths relative to the IDL file being processed. A more complex might search a list of directories, or two lists, one for system schemas and one for application schemas. We might start with just the simple version.

          Show
          Doug Cutting added a comment - A simple include mechanism might just handle paths relative to the IDL file being processed. A more complex might search a list of directories, or two lists, one for system schemas and one for application schemas. We might start with just the simple version.
          Hide
          Doug Cutting added a comment -

          This might work by permitting an include statement in a protocol definition. This would load another protocol definition and add all of its types and methods to the protocol currently being defined.

          Show
          Doug Cutting added a comment - This might work by permitting an include statement in a protocol definition. This would load another protocol definition and add all of its types and methods to the protocol currently being defined.
          Hide
          Doug Cutting added a comment -

          So, here's a concrete proposal: at the start of a protocol definition we permit imports of three different forms, one for an idl file, one for a JSON protocol file and one for a JSON schema file:

          protocol Sample {
            import idl "../../share/authenticate.avdl";
            import protocol "../../share/authorize.avpr";
            import schema "datetime.avsc";
            ...
          }
          

          Quoted paths would be relative to the IDL file being processed. Subsequently, in a separate issue, we might search other directories, and add a syntax for "system" imports, perhaps something like:

          import schema <date.avsc>
          
          Show
          Doug Cutting added a comment - So, here's a concrete proposal: at the start of a protocol definition we permit imports of three different forms, one for an idl file, one for a JSON protocol file and one for a JSON schema file: protocol Sample { import idl "../../share/authenticate.avdl" ; import protocol "../../share/authorize.avpr" ; import schema "datetime.avsc" ; ... } Quoted paths would be relative to the IDL file being processed. Subsequently, in a separate issue, we might search other directories, and add a syntax for "system" imports, perhaps something like: import schema <date.avsc>
          Hide
          Patrick Wendell added a comment -

          This looks great. I would prioritize importing other avdl files first (most common use case) then the others. Right now this is a real blocker in terms of using avro in a large project with several RPC interfaces.

          Show
          Patrick Wendell added a comment - This looks great. I would prioritize importing other avdl files first (most common use case) then the others. Right now this is a real blocker in terms of using avro in a large project with several RPC interfaces.
          Hide
          Doug Cutting added a comment -

          Here's a patch that implements this, with some tests.

          I still need to update the IDL documentation to describe this before it's ready to commit.

          Show
          Doug Cutting added a comment - Here's a patch that implements this, with some tests. I still need to update the IDL documentation to describe this before it's ready to commit.
          Hide
          Doug Cutting added a comment -

          Here's a new version that fixes a bug Patrick Wendell identified offline.

          Show
          Doug Cutting added a comment - Here's a new version that fixes a bug Patrick Wendell identified offline.
          Hide
          Doug Cutting added a comment -

          Here's a new version that updates the IDL documentation.

          Show
          Doug Cutting added a comment - Here's a new version that updates the IDL documentation.
          Hide
          Patrick Wendell added a comment -

          Hey Doug,

          Still getting errors on trying to include a type defined in another file. The import statement itself is fine, but as soon as a type is referenced I get:

          Exception in thread "main" org.apache.avro.idl.ParseException: Undefined name 'com.cloudera.flume.conf.avro.FlumeNodeState' at line 25, column 5
          at org.apache.avro.idl.Idl.ReferenceType(Idl.java:664)
          at org.apache.avro.idl.Idl.Type(Idl.java:584)
          at org.apache.avro.idl.Idl.FieldDeclaration(Idl.java:453)
          at org.apache.avro.idl.Idl.RecordDeclaration(Idl.java:414)
          at org.apache.avro.idl.Idl.NamedSchemaDeclaration(Idl.java:102)
          at org.apache.avro.idl.Idl.ProtocolBody(Idl.java:253)
          at org.apache.avro.idl.Idl.ProtocolDeclaration(Idl.java:162)
          at org.apache.avro.idl.Idl.CompilationUnit(Idl.java:49)
          at org.apache.avro.tool.IdlTool.run(IdlTool.java:65)
          at org.apache.avro.tool.Main.run(Main.java:77)
          at org.apache.avro.tool.Main.main(Main.java:66)

          The relevant file pieces are here:
          [[flumeconfig.avdl]]

          @namespace("com.cloudera.flume.conf.avro")
          protocol FlumeReportAvroServer{
          enum FlumeNodeState { HELLO, IDLE, CONFIGURING, ACTIVE, ERROR, LOST }
          ...

          [[mastercontrol.avdl]]

          @namespace("com.cloudera.flume.conf.avro")
          protocol FlumeMasterAdminServerAvro {
          import protocol "flumeconfig.avdl.avpr";

          record FlumeNodeStatusAvro {
          FlumeNodeState state;

          Show
          Patrick Wendell added a comment - Hey Doug, Still getting errors on trying to include a type defined in another file. The import statement itself is fine, but as soon as a type is referenced I get: Exception in thread "main" org.apache.avro.idl.ParseException: Undefined name 'com.cloudera.flume.conf.avro.FlumeNodeState' at line 25, column 5 at org.apache.avro.idl.Idl.ReferenceType(Idl.java:664) at org.apache.avro.idl.Idl.Type(Idl.java:584) at org.apache.avro.idl.Idl.FieldDeclaration(Idl.java:453) at org.apache.avro.idl.Idl.RecordDeclaration(Idl.java:414) at org.apache.avro.idl.Idl.NamedSchemaDeclaration(Idl.java:102) at org.apache.avro.idl.Idl.ProtocolBody(Idl.java:253) at org.apache.avro.idl.Idl.ProtocolDeclaration(Idl.java:162) at org.apache.avro.idl.Idl.CompilationUnit(Idl.java:49) at org.apache.avro.tool.IdlTool.run(IdlTool.java:65) at org.apache.avro.tool.Main.run(Main.java:77) at org.apache.avro.tool.Main.main(Main.java:66) The relevant file pieces are here: [ [flumeconfig.avdl] ] @namespace("com.cloudera.flume.conf.avro") protocol FlumeReportAvroServer{ enum FlumeNodeState { HELLO, IDLE, CONFIGURING, ACTIVE, ERROR, LOST } ... [ [mastercontrol.avdl] ] @namespace("com.cloudera.flume.conf.avro") protocol FlumeMasterAdminServerAvro { import protocol "flumeconfig.avdl.avpr"; record FlumeNodeStatusAvro { FlumeNodeState state;
          Hide
          Patrick Wendell added a comment -

          Follow-up: was using the wrong version of the patch. Given the most recent version, this works as expected. At least in the use case I am describing above.

          Show
          Patrick Wendell added a comment - Follow-up: was using the wrong version of the patch. Given the most recent version, this works as expected. At least in the use case I am describing above.
          Hide
          Doug Cutting added a comment -

          I committed this.

          Show
          Doug Cutting added a comment - I committed this.

            People

            • Assignee:
              Doug Cutting
              Reporter:
              Eric Evans
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development