[jira] Commented: (AVRO-670) Allow DataFileWriteTool to accept schema files as input

["Philip Zeyliger (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/AVRO-670?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12910712#action_12910712 ] 

Philip Zeyliger commented on AVRO-670:
--------------------------------------

Hi Ron,

The idea and implementation look good.  We should note that it's an incompatible change in terms of the tools command line API, which I think is fine, but should be noted.

I couldn't get the patch to apply cleanly.  Typically (see https://cwiki.apache.org/AVRO/how-to-contribute.html) patches should be generated at top-level (so, lang/java/... should be the path that's being patched).  Many folks name their patches AVRO-670.patch, too, though I'm not a stickler there.  I also recommend turning off Eclipse's autoimport.  I think our style guide discourages star imports (e.g., +import joptsimple.*; is not something your patch should have introduced).  Besides that, "patch" gave me a rejects file---how did you generate your patch?

I was slightly surprised that 'lang/java/src/test/bin/test_tools.sh' (which is run by the ant target 'test-tools') doesn't exercise this code.  Would be good to make sure that the tests for this code don't need any modification.  (There's a java test somewhere, but it might not exercise the command-line parsing code; I haven't looked.)

Could you upload a new patch without the import changes and re-generated against the root of the repo?

Thanks!

> Allow DataFileWriteTool to accept schema files as input
> -------------------------------------------------------
>
>                 Key: AVRO-670
>                 URL: https://issues.apache.org/jira/browse/AVRO-670
>             Project: Avro
>          Issue Type: Improvement
>            Reporter: Ron Bodkin
>             Fix For: 1.5.0
>
>         Attachments: datafilewritefile.patch
>
>
> For non-trivial schemas, it's difficult to pass them inline as a command line argument. I made a patch to use two different arguments: instead of having the first argument be the schema you would now use -schema-file file or -schema schema and then have one other argument (the input JSON file)

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.