Skip to content

Add mysql ssl option #109

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jun 23, 2017
Merged

Conversation

hiroyuki-sato
Copy link
Member

The embulk-input-mysql support SSL connection.
This PR require PR #108.

@hito4t Please take a look when you can.

@hito4t hito4t mentioned this pull request Jun 2, 2017
@hito4t
Copy link
Contributor

hito4t commented Jun 2, 2017

Thank you!
Would you update README?

@hiroyuki-sato
Copy link
Member Author

OK. Thanks. I'll update it later.

@hiroyuki-sato
Copy link
Member Author

@hito4t I updated README. Please take a look when you can.

@hito4t
Copy link
Contributor

hito4t commented Jun 5, 2017

Thank you!
I'm sorry, I had forgotten the following question.
What is the reason for upgrading MySQL connector?

@hiroyuki-sato
Copy link
Member Author

hiroyuki-sato commented Jun 5, 2017

@hito4t I'll double check later.

As far as I remember It's #106 related issue.
I cannot connect mysql server without serverTimezone using Connector/J 5.1.34.
Do I need to create new PR for update Connector/J?

version result options
5.1.34 NG { useLegacyDatetimeCode: false }
5.1.34 OK { useLegacyDatetimeCode: false, serverTimezone: UTC }
5.1.42 OK { useLegacyDatetimeCode: false }

My Tweet (written in Japanese)

Update 2016/06/05 19:35
This is embulk/embulk-output-jdbc#175 issue releated.

And This is the output-jdbc only issue. The input-jdbc issue worked correctly
So It's not important to update Connector/J for input-side

But I personally it is better to use same Connector/J version on the both plugins(input/output).

@hito4t
Copy link
Contributor

hito4t commented Jun 9, 2017

@hiroyuki-sato
Thank you!
The results may be caused by the following change in Connector/J 5.1.35 .

A java.sql.date value was stored incorrectly on the server and also returned incorrectly if the client and the server were in different time zones when useLegacyDatetimeCode=false or useTimezone=true. This was due to the time-zone conversion performed by Connector/J on the SQL DATE type. To avoid the issue, a new property noTimezoneConversionForDateType has been created for Connector/J, which is set to “true” by default, preventing Connector/J to perform the kind of time-zone conversion that caused this bug.

https://dev.mysql.com/doc/relnotes/connector-j/5.1/en/news-5-1-35.html

We should change warning message if updating Connector/J .

You may need to set options useLegacyDatetimeCode and serverTimezone

https://github.com/embulk/embulk-input-jdbc/blob/master/embulk-input-mysql/src/main/java/org/embulk/input/MySQLTimeZoneComparison.java#L60

@hiroyuki-sato
Copy link
Member Author

@hito4t Thank you for your comment.

I'll read noTimezoneConversionForDateType usage.
Let's plan for feature implementation about embulk-ouput-mysql.

What do you think the following plan?

  • First update
    • Introduce comparison timezone between server and client.
    • If both timezones are different, log message useLegacyDatetimeCode and serverTimezone
    • Add warning message like next version will update Connector/J. please take care..
    • Introduce SSL option.
    • Don't update Connector/J
  • Second update (a few week or month later after the first update)
    • Update Connector/J 5.1.42 or later
    • Change the log message useLegacyDatetimeCode and serverTimezone

@hito4t
Copy link
Contributor

hito4t commented Jun 9, 2017

@hiroyuki-sato
Thank you for your suggestion!
I agree.

By the way,

Introduce comparison timezone between server and client.
If both timezones are different, log message useLegacyDatetimeCode and serverTimezone
they were already released in 0.8.3 .

MySQL Connector/J 5.1.35 has incompatibility change about timezone.
It need notify to users before update Connector/J.
@hiroyuki-sato
Copy link
Member Author

hiroyuki-sato commented Jun 10, 2017

@hito4t
I reverted Connector/J version to 5.1.34.
Please take a look again.
I'll separate new PR for output Connector/J version warning.

I've missed seeing 0.8.3 release.

@hito4t hito4t merged commit 491a054 into embulk:master Jun 23, 2017
@hito4t hito4t added this to the 0.8.4 milestone Jun 23, 2017
@hiroyuki-sato
Copy link
Member Author

Thank you!

@hiroyuki-sato hiroyuki-sato deleted the add_mysql_ssl_option branch June 23, 2017 04:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants