feat(csharp/src/Drivers/Apache): Custom ssl server certificate validation for Spark, Impala & Hive #2610
Conversation
CurtHagenlocher
left a comment
CurtHagenlocher
left a comment
There was a problem hiding this comment.
I'm sorry for the delay, and thanks for this change! Most of my feedback is around the user-facing API, as that is harder to change due to backwards-compatibility constraints.
| | `username` | The user name used for basic authentication | | | ||
| | `password` | The password for the user name used for basic authentication. | | | ||
| | `adbc.hive.data_type_conv` | Comma-separated list of data conversion options. Each option indicates the type of conversion to perform on data returned from the Hive server. <br><br>Allowed values: `none`, `scalar`. <br><br>Option `none` indicates there is no conversion from Hive type to native type (i.e., no conversion from String to Timestamp for Apache Hive over HTTP). Example `adbc.hive.conv_data_type=none`. <br><br>Option `scalar` will perform conversion (if necessary) from the Hive data type to corresponding Arrow data types for types `DATE/Date32/DateTime`, `DECIMAL/Decimal128/SqlDecimal`, and `TIMESTAMP/Timestamp/DateTimeOffset`. Example `adbc.hive.conv_data_type=scalar` | `scalar` | | ||
| | `adbc.hive.tls_options` | Comma-separated list of TLS/SSL options. Each option indicates the TLS/SSL option when connecting to a Hive server. <br><br>Allowed values: `allow_self_signed`, `allow_hostname_mismatch`. <br><br>Option `allow_self_signed` allows certificate errors due to an unknown certificate authority, typically when using a self-signed certificate. Option `allow_hostname_mismatch` allow certificate errors due to a mismatch of the hostname. (e.g., when connecting through an SSH tunnel). Example `adbc.hive.tls_options=allow_self_signed` | | |
There was a problem hiding this comment.
It would be nice and relatively low-cost to stay backwards-compatible by continuing to support the existing option for another release before removing it. (We haven't declared "1.0" yet so I don't think it's strictly necessary, but I think erring on the side of accommodating consumption scenarios is broadly good.)
There was a problem hiding this comment.
Having both the options would be confusing to the user. Also there is a dilemma on which option to pick if both options are set. Isn't it?
There was a problem hiding this comment.
It would be temporary -- for one release -- and specifying both should result in an error. The idea is that having one published version that supports both the old and the new options lets someone update their code independently of the dependency -- instead of having to update multiple things at once. Then they just need to update their own code before the very next release to avoid being broken..
I don't think this is critical (because we're not yet calling the version "1.0"), but if this were a "GA product" then changing the connection string options would be very impactful because connection strings have a lifetime that's often disconnected from the code that's consuming the driver.
CurtHagenlocher
left a comment
CurtHagenlocher
left a comment
There was a problem hiding this comment.
Thanks for the changes! I leave it up to you whether to retain backwards-compatibility, but the one wrong value in the README should definitely be updated.
| | `username` | The user name used for basic authentication | | | ||
| | `password` | The password for the user name used for basic authentication. | | | ||
| | `adbc.hive.data_type_conv` | Comma-separated list of data conversion options. Each option indicates the type of conversion to perform on data returned from the Hive server. <br><br>Allowed values: `none`, `scalar`. <br><br>Option `none` indicates there is no conversion from Hive type to native type (i.e., no conversion from String to Timestamp for Apache Hive over HTTP). Example `adbc.hive.conv_data_type=none`. <br><br>Option `scalar` will perform conversion (if necessary) from the Hive data type to corresponding Arrow data types for types `DATE/Date32/DateTime`, `DECIMAL/Decimal128/SqlDecimal`, and `TIMESTAMP/Timestamp/DateTimeOffset`. Example `adbc.hive.conv_data_type=scalar` | `scalar` | | ||
| | `adbc.hive.tls_options` | Comma-separated list of TLS/SSL options. Each option indicates the TLS/SSL option when connecting to a Hive server. <br><br>Allowed values: `allow_self_signed`, `allow_hostname_mismatch`. <br><br>Option `allow_self_signed` allows certificate errors due to an unknown certificate authority, typically when using a self-signed certificate. Option `allow_hostname_mismatch` allow certificate errors due to a mismatch of the hostname. (e.g., when connecting through an SSH tunnel). Example `adbc.hive.tls_options=allow_self_signed` | | |
There was a problem hiding this comment.
It would be temporary -- for one release -- and specifying both should result in an error. The idea is that having one published version that supports both the old and the new options lets someone update their code independently of the dependency -- instead of having to update multiple things at once. Then they just need to update their own code before the very next release to avoid being broken..
I don't think this is critical (because we're not yet calling the version "1.0"), but if this were a "GA product" then changing the connection string options would be very impactful because connection strings have a lifetime that's often disconnected from the code that's consuming the driver.
…tion for Spark, Impala & Hive (apache#2610) Co-authored-by: Sudhir Emmadi <[email protected]>
3 participants
No description provided.