Avro
  1. Avro
  2. AVRO-1397

Binary fragment tools should allow more than one datum or JSON object

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.7.5
    • Fix Version/s: 1.7.6
    • Component/s: java
    • Labels:
      None

      Description

      It would be useful for the binary fragment tools to process more than one datum or JSON object.

      1. AVRO-1397.patch
        16 kB
        Rob Turner
      2. AVRO-1397.patch
        6 kB
        Doug Cutting
      3. AVRO-1397.patch
        7 kB
        Rob Turner

        Activity

        Rob Turner created issue -
        Hide
        Rob Turner added a comment -

        Please review this patch that resolves this issue.

        Show
        Rob Turner added a comment - Please review this patch that resolves this issue.
        Rob Turner made changes -
        Field Original Value New Value
        Attachment AVRO-1397.patch [ 12615423 ]
        Hide
        Doug Cutting added a comment -

        Here's a version of the patch that removes whitespace-only changes. This let's me focus on the functional changes under review.

        BinaryFragmentToJsonTool no longer seems to pretty-print. That would be an incompatible change. Perhaps there should be a command-line option to disable pretty-printing, in order to maintain compatibility?

        JsonToBinaryFragmentTool may no longer signal an error if the file contains a truncated json expression, e.g., missing a final bracket. Instead, such expressions may be silently dropped. Is that right? If so, is there a way to fix it?

        Thanks!

        Show
        Doug Cutting added a comment - Here's a version of the patch that removes whitespace-only changes. This let's me focus on the functional changes under review. BinaryFragmentToJsonTool no longer seems to pretty-print. That would be an incompatible change. Perhaps there should be a command-line option to disable pretty-printing, in order to maintain compatibility? JsonToBinaryFragmentTool may no longer signal an error if the file contains a truncated json expression, e.g., missing a final bracket. Instead, such expressions may be silently dropped. Is that right? If so, is there a way to fix it? Thanks!
        Doug Cutting made changes -
        Attachment AVRO-1397.patch [ 12615753 ]
        Hide
        Rob Turner added a comment -

        Sorry about the whitespace changes, is there a formatter for Eclipse?

        Corrected Patch

        Corrected my initial patch so that BinaryFragmentToJsonTool now pretty prints by default and I have added a --no-pretty option. I also added the --schema-file option to both BinaryFragmentToJsonTool and JsonToBinaryFragmentTool.

        JSON Pretty Printing

        I added a method EncoderFactory.jsonEncoder(Schema schema, OutputStream out, boolean pretty) and changed JsonEncoder to create an extended DefaultPrettyPrinter that uses a line.separator as the root value separator rather than a space that is hard coded in DefaultPrettyPrinter. This could be used to provide a pretty print option for the tojson tool AVRO-1396.

        JsonToBinaryFragmentTool Catches EOFException

        This appears to be the only way to detect the end of the file, it is only thrown if the JSON parser has no more tokens and the stack depth is at the root level. A truncated JSON object throws a JsonParseException. The fromjson tool uses this method too.

        Please review this patch.

        Show
        Rob Turner added a comment - Sorry about the whitespace changes, is there a formatter for Eclipse? Corrected Patch Corrected my initial patch so that BinaryFragmentToJsonTool now pretty prints by default and I have added a --no-pretty option. I also added the --schema-file option to both BinaryFragmentToJsonTool and JsonToBinaryFragmentTool. JSON Pretty Printing I added a method EncoderFactory.jsonEncoder(Schema schema, OutputStream out, boolean pretty) and changed JsonEncoder to create an extended DefaultPrettyPrinter that uses a line.separator as the root value separator rather than a space that is hard coded in DefaultPrettyPrinter. This could be used to provide a pretty print option for the tojson tool AVRO-1396 . JsonToBinaryFragmentTool Catches EOFException This appears to be the only way to detect the end of the file, it is only thrown if the JSON parser has no more tokens and the stack depth is at the root level. A truncated JSON object throws a JsonParseException. The fromjson tool uses this method too. Please review this patch.
        Rob Turner made changes -
        Attachment AVRO-1398.patch [ 12616148 ]
        Hide
        Rob Turner added a comment -

        Sorry, previous patch had the wrong name this one is correct.

        Show
        Rob Turner added a comment - Sorry, previous patch had the wrong name this one is correct.
        Rob Turner made changes -
        Attachment AVRO-1397.patch [ 12616300 ]
        Rob Turner made changes -
        Attachment AVRO-1398.patch [ 12616148 ]
        Rob Turner made changes -
        Assignee Rob Turner [ robair ]
        Hide
        Doug Cutting added a comment -

        This is looking good. Sorry it took me so long to get back to it!

        For compatibility, pretty-printing should be off by default, so the option should be --pretty to enable it.

        Other than that, +1.

        Show
        Doug Cutting added a comment - This is looking good. Sorry it took me so long to get back to it! For compatibility, pretty-printing should be off by default, so the option should be --pretty to enable it. Other than that, +1.
        Hide
        Rob Turner added a comment -

        Doug you mentioned above on 26/Nov/13 00:

        BinaryFragmentToJsonTool no longer seems to pretty-print. That would be an incompatible change.

        so that's why I have left pretty-printing on by default. I can swap this if you wish.

        Show
        Rob Turner added a comment - Doug you mentioned above on 26/Nov/13 00: BinaryFragmentToJsonTool no longer seems to pretty-print. That would be an incompatible change. so that's why I have left pretty-printing on by default. I can swap this if you wish.
        Hide
        ASF subversion and git services added a comment -

        Commit 1550850 from Doug Cutting in branch 'avro/trunk'
        [ https://svn.apache.org/r1550850 ]

        AVRO-1397. Java: Binary fragment tools can now read multiple objects from their input. Contributed by Rob Turner.

        Show
        ASF subversion and git services added a comment - Commit 1550850 from Doug Cutting in branch 'avro/trunk' [ https://svn.apache.org/r1550850 ] AVRO-1397 . Java: Binary fragment tools can now read multiple objects from their input. Contributed by Rob Turner.
        Hide
        Doug Cutting added a comment -

        Sorry! I got confused.

        I committed this. Thanks, Rob!

        Show
        Doug Cutting added a comment - Sorry! I got confused. I committed this. Thanks, Rob!
        Doug Cutting made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Fix Version/s 1.7.6 [ 12324942 ]
        Resolution Fixed [ 1 ]
        Hide
        Hudson added a comment -

        SUCCESS: Integrated in AvroJava #407 (See https://builds.apache.org/job/AvroJava/407/)
        AVRO-1397. Java: Binary fragment tools can now read multiple objects from their input. Contributed by Rob Turner. (cutting: rev 1550850)

        • /avro/trunk/CHANGES.txt
        • /avro/trunk/lang/java/tools/src/main/java/org/apache/avro/tool/BinaryFragmentToJsonTool.java
        • /avro/trunk/lang/java/tools/src/main/java/org/apache/avro/tool/JsonToBinaryFragmentTool.java
        • /avro/trunk/lang/java/tools/src/test/java/org/apache/avro/tool/TestJsonToFromBinaryFragmentTools.java
        Show
        Hudson added a comment - SUCCESS: Integrated in AvroJava #407 (See https://builds.apache.org/job/AvroJava/407/ ) AVRO-1397 . Java: Binary fragment tools can now read multiple objects from their input. Contributed by Rob Turner. (cutting: rev 1550850) /avro/trunk/CHANGES.txt /avro/trunk/lang/java/tools/src/main/java/org/apache/avro/tool/BinaryFragmentToJsonTool.java /avro/trunk/lang/java/tools/src/main/java/org/apache/avro/tool/JsonToBinaryFragmentTool.java /avro/trunk/lang/java/tools/src/test/java/org/apache/avro/tool/TestJsonToFromBinaryFragmentTools.java
        Doug Cutting made changes -
        Status Resolved [ 5 ] Closed [ 6 ]

          People

          • Assignee:
            Rob Turner
            Reporter:
            Rob Turner
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development