search

embulk / embulk-input-jdbc

MySQL, PostgreSQL, Redshift and generic JDBC input plugins for Embulk
Other
102 stars 74 forks source link

Replace Joda-Time DateTimeZone to Java 8 java.time classes #181

Closed dmikurube closed 4 years ago

dmikurube commented 4 years ago

Along with making JRuby optional in Embulk, Joda-Time would be also deprecated. Java 8's java.time classes can replace them.

hito4t commented 4 years ago

@hito4t Thanks for your comment. Good catch! It still fails, but during the run, not at the configuration step with ConfigException.

It should fail earlier at the configuration step. Changed to do so in: b64e1e9

Time zone is not only for embulk-input-jdbc. Many plugins will use time zone. I think embulk-core instead of embulk-input-jdbc should check time zone in the config.

How about using ZoneId instead of String to represent time zone in the config, and creating serializer/deserializer for ZoneId in embulk-core?

dmikurube commented 4 years ago

@hito4t That kind of special data conversion in org.embulk.config.* has made the config processing situation complicated in embulk-core (especially about dependency libraries and ClassLoaders).

So, the embulk-core team has decided not to support more conversions, and furthermore, deprecate some existing. Sorry for inconvenience.

hito4t commented 4 years ago

@dmikurube

I understand embulk-core can't check time zone.

I'm afraid other plugins would forget to check time zone, but I don't have a good idea.

By the way, could you add tests that have invalid time zone in the config?

dmikurube commented 4 years ago

@hito4t (Sorry to be so late.) Thanks for your comment. Added a test in: 746761f0379c3c58413a23ba86e7a7dc3cf0846d

For the future, we're preparing a plugin-side library embulk-util-config, which is to process Embulk configs on plugin's side, replacing core-side ConfigSource#loadConfig and TaskSource#loadTask. https://github.com/embulk/embulk-util-config

In this library, a plugin can configure its own Jackson SerDe and validation in it. Once the library is available widely, a similar validation would be available again running on plugin's side. (But, we'll need to cleanup embulk-core's Jackson and around before that.)

dmikurube commented 4 years ago

Thanks!