avro-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bridger Howell (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (AVRO-2079) Add ability to use Java 8 date/time types instead of Joda time.
Date Sun, 15 Oct 2017 23:11:00 GMT

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

Bridger Howell commented on AVRO-2079:
--------------------------------------

[~aukevanleeuwen]: I've looked over your PR and left a few nits a bit ago (under the username
"Kuroshii").

Upon revisiting, I have some higher-level thoughts:
1. It seems weird how, after this PR, the configuration for {{SpecificCompiler}} would be
mixed between a constructor parameter and setters. If we're going to take this approach, my
preference would be to isolate all the compiler configuration into it's own immutable {{SpecificCompilerOptions}}
class (and associated builder) that's set once in the constructor. Not that you should refactor
everything now, but if you're lobbying for switching to a different convention, we should
create a separate ticket and deal with it there.

2. It's nice that you have a lot of tests to be sure that these time conversions won't break
later, but I dislike how we're expanding the amount that we have to check that the compiler
output precisely matches predetermined files. This is brittle and makes much harder to change
{{SpecificCompiler}} for unrelated changes. Worst case, when we add new options, we start
considering every combination of compiler flags and eventually end up with an explosion of
brittle test cases. I'm not sure of a great solution to this yet, but just something I'd like
to keep in mind.

3. I would have liked this to handle this situation in a more general way.
* Set a particular compiler option
* Setting that option adds those conversions to the {{SpecificCompiler}} class which in turn
makes those conversions generally available to the generated code

This last point might seem minor, but I think it would do a lot enable future configuration
(e.g. users being able to add their own third-party conversions to generated classes without
needing the {{SpecificCompiler}} to have already been prepared for those cases).

Also, I'd appreciate it if a committer could have a look at these changes soon. This topic
is very significant to the way I use avro, and I'm not very familiar with all the particular
usages of {{SpecificCompiler}}. 

> Add ability to use Java 8 date/time types instead of Joda time.
> ---------------------------------------------------------------
>
>                 Key: AVRO-2079
>                 URL: https://issues.apache.org/jira/browse/AVRO-2079
>             Project: Avro
>          Issue Type: Improvement
>          Components: java, logical types
>    Affects Versions: 1.8.2
>            Reporter: Auke van Leeuwen
>              Labels: patch-available
>
> Currently, for the date/time related logical types, we are generating Joda date/time
objects. Since we've moved to Java-8 (AVRO-2043) it seems logical to also provide the possibility
to generate {{java.time.*}} date/time objects instead of the Joda time variants.
> I propose to make this is a switch in {{SpecificCompiler.java}} which will default to
Joda (I think), but can be set to generate the Java 8 versions.
> (I'm currently trying to run through the code to see if I can make it work.)



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Mime
View raw message