Output a version of MySQL Connector/J. #110

hiroyuki-sato · 2017-06-10T09:51:21Z

Overview

I would like to update MySQL Connector/J to a newer version. (5.1.42 or newer)
But It has incompatibility change.
We(I and @hito4t) decided that the plugin output a warning message like This plugin will update Connector/J on next release...
This PR for implement this warning.

Detail

@hito4t, Could you give me some advice?

When output a version information and warning?
How to output current Connector/J version.
- How to get current Driver version (hard code, get version info from libraries)
How to implement it.

When output a version information and warning?

I think I should the message should output before Loaded plugin embulk/input/mysql from a load path.

Example.

2017-06-10 11:04:02.020 +0900: Embulk v0.8.22
2017-06-10 11:04:14.232 +0900 [INFO] (0001:preview): MySQL Connector/J version 5.1.34
2017-06-10 11:04:13.835 +0900 [INFO] (0001:preview): Loaded plugin embulk/input/mysql from a load path
.... and some warning message here

How to output current Connector/J version?

I would like to output warning message with current MySQL Connector/J version.
So I tried to get it.

The current experimental implementation get version info from Package object.

First I tried to get the version info from Driver class.
But It can't get sub minor version (z part in x.y.z).
That's why I used this way.

Do you have any alternative idea?

Driver driver;
try {
    driver = new com.mysql.jdbc.Driver();

    int major = driver.getMajorVersion();
    int minor = driver.getMinorVersion();
    logger.info(String.format(Locale.ENGLISH,"driver version %d.%d",major,minor));
}
catch (SQLException e) {
    throw Throwables.propagate(e);
}

hito4t · 2017-06-12T01:03:46Z

Thank you for the PR!

How to output current Connector/J version.
How to get current Driver version (hard code, get version info from libraries)

How to implement it.

We can get driver information by java.sql.DatabaseMetaData object gotten by java.sql.Connection#getMetaData() .

DatabaseMetaData of MySQL Connector/J will return following values.

getDriverMajorVersion() -> 5
getDriverMinorVersion() -> 1
getDriverName() -> "MySQL Connector Java"
getDriverVersion() -> "mysql-connector-java-5.1.34 ( Revision: [email protected] )"

We should output a result of getDriverVersion() because other methods won't return enough information.

When output a version information and warning?

I think we should do in setupTask method before checking timezone.

And it must be useful if driver version is shown in not also embulk-output-mysql but also all embulk-output-jdbc plugins.
For example, we can do in AbstractJdbcInputPlugin#transaction method before calling setupTask .

This reverts commit d0ebcd1.

Use java.sql.Connection#getMetaData() method to output JDBC driver version.

hiroyuki-sato · 2017-06-12T05:04:42Z

@hito4t Thank you for your comment.

I used java.sql.Connection#getMetaData()
What do you think this implementation?

I'll change later the following part
`This plugin will update MySQL Connector/J version on next release...``

hito4t

Thank you!
Please check comments.

hito4t · 2017-06-14T01:18:47Z

embulk-input-jdbc/src/main/java/org/embulk/input/jdbc/JdbcInputConnection.java

+        try {
+            DatabaseMetaData meta = connection.getMetaData();
+            logger.info(String.format(Locale.ENGLISH,"Using JDBC Driver %s",meta.getDriverVersion()));
+        } catch( SQLException e ) {


I think we don't need to catch SQLException because Connection#getMetaData#getDriverVersion doesn't throw an exception normally.
If thrown, we can't access database after all.

Throw the exception instead of catch.

hito4t · 2017-06-14T01:22:50Z

embulk-input-jdbc/src/main/java/org/embulk/input/jdbc/AbstractJdbcInputPlugin.java

@@ -185,6 +185,7 @@ public ConfigDiff transaction(ConfigSource config,
        Schema schema;
        try (JdbcInputConnection con = newConnection(task)) {
            // TODO incremental_columns is not set => get primary key
+            con.showDriverVersion();


The comment above (// TODO incremental_columns ...) should be above the following line (schema = setupTask(con, task);).

Move comment to propper place.

hiroyuki-sato · 2017-06-14T05:14:22Z

@hito4t Thank you for reviewing.

Please take a look again.
I fixed your review and add log detail.

It output the message like the below.

2017-06-14 14:11:22.435 +0900 [WARN] (0001:preview): This plugin will update MySQL Connector/J version on a future release.
2017-06-14 14:11:22.435 +0900 [WARN] (0001:preview): It has some incompatibility change.
2017-06-14 14:11:22.435 +0900 [WARN] (0001:preview): Please read a document and make sure configuration carefully before update the plugin.

hito4t · 2017-06-15T04:52:34Z

Thank you!
Shall we elaborate the waning message?

All plugins must upgrade drivers one day. I think "near future" is better than "future".
It would be useful if users could know what "incompatibility" is (as written in Add mysql ssl option #109 (comment)).
I think "Please read a document ..." is not needed because users should read a document even if driver is not upgraded.

hiroyuki-sato · 2017-06-15T15:09:43Z

@hito4t
Thank you!
What do you think this message?
I read release note again. It seems that the incompatibility change are noTimezoneConversionForDateType and cacheDefaultTimezone only.

This plugin will update MySQL Connector/J version in the near future release.
It has some incompatibility change.
For example, the 5.1.35 introduce noTimezoneConversionForDateType and cacheDefaultTimezone options.
Please confirm your configuration before update the plugin.

hito4t · 2017-06-20T05:19:33Z

@hiroyuki-sato
Thank you!
That's nice.
Please commit the source after correcting errors.
some incompatibility change -> some incompatibility changes
the 5.1.35 introduce -> the 5.1.35 introduced
before update -> before updating

hiroyuki-sato · 2017-06-20T05:48:24Z

@hito4t

Thank you for your comment!.

I fixed some errors.
Please take a look again when you can.

2017-06-20 14:44:16.325 +0900 [WARN] (0001:preview): This plugin will update MySQL Connector/J version in the near future release.
2017-06-20 14:44:16.325 +0900 [WARN] (0001:preview): It has some incompatibility changes.
2017-06-20 14:44:16.325 +0900 [WARN] (0001:preview): For example, the 5.1.35 introduced noTimezoneConversionForDateType and cacheDefaultTimezone options.
2017-06-20 14:44:16.325 +0900 [WARN] (0001:preview): Please read a document and make sure configuration carefully before updating the plugin.

hiroyuki-sato · 2017-06-20T09:57:48Z

Thank you!

hito4t · 2017-06-23T07:55:35Z

I'd like to resolve #113 before upgrading MySQL Connector/J.

Output a version of MySQL Connector/J.(Experimental)

d0ebcd1

hiroyuki-sato added 3 commits June 12, 2017 13:09

Revert "Output a version of MySQL Connector/J.(Experimental)"

0f893f6

This reverts commit d0ebcd1.

Use Connection#getMetaData() method.

fda881a

Use java.sql.Connection#getMetaData() method to output JDBC driver version.

Output warning message about update Connector/J

4669b1e

hito4t reviewed Jun 14, 2017

View reviewed changes

hiroyuki-sato added 3 commits June 14, 2017 14:07

Move comment to proper place

822642a

Throw SQLException instead of catch

aa1d22b

Write warning detail

c073d40

Fix MySQL driver warning message

b3d85bf

hito4t merged commit 80a3343 into embulk:master Jun 20, 2017

hiroyuki-sato deleted the mysql_jdbc_version branch June 20, 2017 09:57

hito4t changed the title ~~[WIP] Output a version of MySQL Connector/J.~~ Output a version of MySQL Connector/J. Jun 21, 2017

hito4t added this to the 0.8.4 milestone Jun 23, 2017

hiroyuki-sato mentioned this pull request Jun 25, 2017

Output JDBC driver version and MySQL Connector/J update warning embulk/embulk-output-jdbc#182

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Output a version of MySQL Connector/J. #110

Output a version of MySQL Connector/J. #110

hiroyuki-sato commented Jun 10, 2017

hito4t commented Jun 12, 2017

hiroyuki-sato commented Jun 12, 2017

hito4t left a comment

hito4t Jun 14, 2017

hiroyuki-sato Jun 14, 2017

hito4t Jun 14, 2017

hiroyuki-sato Jun 14, 2017 •

edited

Loading

hiroyuki-sato commented Jun 14, 2017

hito4t commented Jun 15, 2017

hiroyuki-sato commented Jun 15, 2017

hito4t commented Jun 20, 2017

hiroyuki-sato commented Jun 20, 2017 •

edited

Loading

hiroyuki-sato commented Jun 20, 2017

hito4t commented Jun 23, 2017

Output a version of MySQL Connector/J. #110

Output a version of MySQL Connector/J. #110

Conversation

hiroyuki-sato commented Jun 10, 2017

Overview

Detail

When output a version information and warning?

How to output current Connector/J version?

hito4t commented Jun 12, 2017

hiroyuki-sato commented Jun 12, 2017

hito4t left a comment

Choose a reason for hiding this comment

hito4t Jun 14, 2017

Choose a reason for hiding this comment

hiroyuki-sato Jun 14, 2017

Choose a reason for hiding this comment

hito4t Jun 14, 2017

Choose a reason for hiding this comment

hiroyuki-sato Jun 14, 2017 • edited Loading

Choose a reason for hiding this comment

hiroyuki-sato commented Jun 14, 2017

hito4t commented Jun 15, 2017

hiroyuki-sato commented Jun 15, 2017

hito4t commented Jun 20, 2017

hiroyuki-sato commented Jun 20, 2017 • edited Loading

hiroyuki-sato commented Jun 20, 2017

hito4t commented Jun 23, 2017

hiroyuki-sato Jun 14, 2017 •

edited

Loading

hiroyuki-sato commented Jun 20, 2017 •

edited

Loading