sadpandajoe commented on code in PR #43695:
URL: https://github.com/apache/superset/pull/43695#discussion_r4272178212
##########
superset/db_engine_specs/databricks.py:
##########
@@ -1011,6 +1019,56 @@ def build_sqlalchemy_uri( # type: ignore
query=query,
).render_as_string(hide_password=False)
+ @classmethod
+ def adjust_engine_params(
+ cls,
+ uri: URL,
+ connect_args: dict[str, Any],
+ catalog: str | None = None,
+ schema: str | None = None,
+ ) -> tuple[URL, dict[str, Any]]:
+ adjusted_connect_args = connect_args.copy()
+
+ is_catalog_overridden = catalog is not None
+ effective_catalog = (
+ catalog
+ or adjusted_connect_args.get("catalog")
+ or uri.query.get("catalog")
+ )
+ effective_schema = (
+ schema
+ or (adjusted_connect_args.get("schema") if not
is_catalog_overridden else None)
+ or (uri.query.get("schema") if not is_catalog_overridden else None)
+ or (uri.database if uri.database and not is_catalog_overridden
else None)
+ )
+ effective_http_path = (
+ adjusted_connect_args.get("http_path")
+ or uri.query.get("http_path")
+ )
+
+ query_dict = dict(uri.query)
+ if effective_catalog:
+ query_dict["catalog"] = effective_catalog
+ adjusted_connect_args["catalog"] = effective_catalog
+ else:
+ query_dict.pop("catalog", None)
+ adjusted_connect_args.pop("catalog", None)
+
+ if effective_schema:
+ query_dict["schema"] = effective_schema
+ adjusted_connect_args["schema"] = effective_schema
+ else:
+ query_dict.pop("schema", None)
Review Comment:
When only a catalog override is passed (for example creating a dataset with
`catalog="main"`, `schema=None` against a `/default?catalog=main&http_path=...`
URI), this branch drops `schema` from both the query and `connect_args`, so the
aliased dialect ends up with `self.schema = None`. Primary-key and foreign-key
reflection then builds ``DESCRIBE TABLE EXTENDED `main`.`None`.`orders` `` and
rejects a table that exists as `main.default.orders`. Should the cleared-schema
case resolve the catalog's default schema (or avoid emitting a literal `None`)
for reflection?
##########
superset/db_engine_specs/lint_metadata.py:
##########
@@ -605,11 +643,18 @@ def generate_markdown_report(reports:
list[MetadataReport]) -> str:
"",
]
)
-
return "\n".join(lines)
-def main() -> int:
+def main() -> int: # noqa: C901
+ """Main entry point."""
+ if hasattr(sys.stdout, "reconfigure"):
Review Comment:
The stdout/stderr UTF-8 reconfiguration here has no test coverage:
`test_lint_metadata.py` only exercises AST discovery, so removing these lines
keeps the suite green while the report's Unicode symbols raise
`UnicodeEncodeError` on an ASCII-configured console. Could we add a test that
calls `main()` against one temporary spec with a `TextIOWrapper` stdout
configured as ASCII and asserts it completes with UTF-8-decodable output?
--
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]
