ClickHouse / clickhouse-connect

Python driver/sqlalchemy/superset connectors
Apache License 2.0
332 stars 65 forks source link

small suggestion - port int coercion in get_client #395

Closed vicentesurraco closed 2 months ago

vicentesurraco commented 2 months ago

I was struggling to figure out why I could connect to clickhouse everywhere except clickhouse-client. It turned out the port in my env was being read as a string and it must be passed in as an int to get_client if the port is 443 or 8443. This is because use_tls returns False when comparing "8443" to 8443. Then the interface is set to http instead of https and get_client responds with ConnectionResetError: [Errno 54] Connection reset by peer. It would have been nice if it coerced the port to an int (or at least thrown a clearer error).

The following change would fix this issue (wrapping port with int())

use_tls = str(secure).lower() == 'true' or interface == 'https' or (not interface and int(port) in (443, 8443))

Figured I might as well suggest this in case it helps someone in the future.

genzgd commented 2 months ago

I agree with the suggestion but you could also have set the interface to https explicitly.