search

embulk / embulk-input-jdbc

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

Update ssl option doc README #172

Closed y-ken closed 4 years ago

y-ken commented 4 years ago

There are ssl option implementation but README does not have the information. https://github.com/embulk/embulk-input-jdbc/blob/master/embulk-input-mysql/src/main/java/org/embulk/input/MySQLInputPlugin.java#L50-L52

y-ken commented 4 years ago

@sakama or @hito4t could you please check this?

hito4t commented 4 years ago

@y-ken Thank you for the PR!

The ssl option of embulk-input-mysql is same as the ssl option of embulk-output-mysql. How about copying readme from embulk-output-mysql? https://github.com/embulk/embulk-output-jdbc/blob/master/embulk-output-mysql/README.md

- **ssl**: use SSL to connect to the database (string, default: `disable`. `enable` uses SSL without server-side validation and `verify` checks the certificate. For compatibility reasons, `true` behaves as `enable` and `false` behaves as `disable`.
y-ken commented 4 years ago

@hito4t Got it. After that, all of readme has these ssl option description.

$ grep ssl embulk-input-*/README.md
embulk-input-jdbc/README.md:- **ssl**: use SSL to connect to the database (string, default: `disable`. `enable` uses SSL without server-side validation and `verify` checks the certificate. For compatibility reasons, `true` behaves as `enable` and `false` behaves as `disable`.)
embulk-input-mysql/README.md:- **ssl**: use SSL to connect to the database (string, default: `disable`. `enable` uses SSL without server-side validation and `verify` checks the certificate. For compatibility reasons, `true` behaves as `enable` and `false` behaves as `disable`.)
embulk-input-postgresql/README.md:- **ssl**: enables SSL. Data will be encrypted but CA or certification will not be verified (boolean, default: false)
embulk-input-redshift/README.md:- **ssl**: enables SSL. Data will be encrypted but CA or certification will not be verified (boolean, default: false)
hito4t commented 4 years ago

@y-ken Thank you!

The type of the ssl option in embulk-input-postgresql and embulk-input-redshift is boolean, although the type of the ssl option in embulk-input-mysql is enum Ssl. And because embulk-input-jdbc doesn't have ssl option, we should remove it from the readme.

y-ken commented 4 years ago

oh, does it need to remove ssl option line from README for embulk-input-jdbc?

embulk-input-jdbc/README.md:- **ssl**: use SSL to connect to the database (string, default: `disable`. `enable` uses SSL without server-side validation and `verify` checks the certificate. For compatibility reasons, `true` behaves as `enable` and `false` behaves as `disable`.)
hito4t commented 4 years ago

@y-ken Yes, neither JdbcInputPlugin.GenericPluginTask interface nor its super interface (AbstractJdbcInputPlugin.InputPlugin) have the ssl property. Would you update README for embulk-input-jdbc?

y-ken commented 4 years ago

@hito4t it seems some results, but it does not effect to use ssl. does it correct?

$ grep -r -i "ssl" ./embulk-input-jdbc/
./embulk-input-jdbc//src/main/java/org/embulk/input/jdbc/Ssl.java:public enum Ssl
./embulk-input-jdbc//src/main/java/org/embulk/input/jdbc/Ssl.java:    public static Ssl fromString(String value)
./embulk-input-jdbc//src/main/java/org/embulk/input/jdbc/Ssl.java:            throw new ConfigException(String.format("Unknown SSL value '%s'. Supported values are enable, true, disable, false or verify.", value));
hito4t commented 4 years ago

@y-ken Thank you!

it seems some results, but it does not effect to use ssl. does it correct?

embulk-input-jdbc/src/main/java/org/embulk/input/jdbc/Ssl.java is a class shared among embulk-input-jdbc plugins. But embulk-input-jdbc itself doesn't use Ssl class.