rad-pat commented on code in PR #28627:
URL: https://github.com/apache/superset/pull/28627#discussion_r3728310369
##########
superset/db_engine_specs/databend.py:
##########
@@ -217,10 +217,11 @@ class DatabendConnectEngineSpec(BasicParametersMixin,
DatabendEngineSpec):
_function_names: list[str] = []
sqlalchemy_uri_placeholder = (
-
"databend://user:password@host[:port][/dbname][?secure=value&=value...]"
+
"databend://user:password@host[:port][/dbname][?sslmode=value&=value...]"
)
parameters_schema = DatabendParametersSchema()
- encryption_parameters = {"secure": "true"}
+ encryption_parameters = {"sslmode": "require"}
+ encryption_disable_parameters = {"sslmode": "disable"}
Review Comment:
Done — `encryption_disable_parameters` is now on `BasicParametersMixin`,
with `build_sqlalchemy_uri` applying it on the `else` branch and
`get_parameters_from_uri` stripping it. It defaults to `{}`, so
`query.update({})` is a no-op and no other spec changes behaviour.
To answer my own question from last time: there's no need for an inverse
boolean. `encryption` still derives from `encryption_parameters` alone — the
disable set's only job in the read path is to be *removed*, so it doesn't
survive into `query` and reappear as a user-supplied extra parameter. Once I
framed it that way the asymmetry I was stuck on went away.
One subtlety worth flagging for whoever reviews it: the stripping can't
merge the two dicts. They typically share a key with *different* values
(`sslmode=require` / `sslmode=disable`), so `{**enable, **disable}` would keep
only one and leak the other. It builds a list of `.items()` pairs from both
instead.
`DatabendEngineSpec` still overrides both methods, because it needs the
`__default__` database substitution, the legacy `secure` fallback, and a bare
`databend://` scheme (the mixin builds `engine+default_driver`, which here
would be `databend+databend://` and would change every stored URI). So the
mixin change is generic groundwork rather than something Databend routes
through — happy to take it back out if you'd rather it landed with its first
real consumer.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
