dwreeves commented on code in PR #31846:
URL: https://github.com/apache/airflow/pull/31846#discussion_r1228511956
##########
airflow/providers/databricks/hooks/databricks_sql.py:
##########
@@ -138,15 +141,39 @@ def get_conn(self) -> Connection:
)
return self._sql_conn
+ @overload
+ def run(
+ self,
+ sql: str | Iterable[str],
+ autocommit: bool = False,
+ parameters: Iterable | Mapping | None = None,
+ handler: None = None,
+ split_statements: bool = True,
+ return_last: bool = True,
+ ) -> None:
+ ...
+
+ @overload
+ def run(
+ self,
+ sql: str | Iterable[str],
+ autocommit: bool = False,
+ parameters: Iterable | Mapping | None = None,
+ handler: Callable[[Any], T] = None, # type: ignore[assignment]
Review Comment:
> So is it the other way around, for the `T | list[T]` variant, handler must
be set to a callable right?
Yes.
> I think in that case I’d perfer we simply remove the argument default from
the overload signature altogether. Maybe also make it keyword-only (not
technically perfect but practically that’s how the argument should be specified
anyway).
The problem with doing that is Mypy doesn't allow for it. The
`mypy-providers` returns 18 errors if I do that:
```
Found 18 errors in 16 files (checked 993 source files)
```
Here is what I replaced each `run` typing with (+ Snowflake one has extra
kwarg).
```python
@overload
def run(
self,
sql: str | Iterable[str],
*,
autocommit: bool,
parameters: Iterable | Mapping[str, Any] | None,
handler: None,
split_statements: bool,
return_last: bool,
) -> None:
...
@overload
def run(
self,
sql: str | Iterable[str],
*,
autocommit: bool,
parameters: Iterable | Mapping[str, Any] | None,
handler: Callable[[Any], T],
split_statements: bool,
return_last: bool,
) -> T | list[T]:
...
def run(
self,
sql: str | Iterable[str],
autocommit: bool = False,
parameters: Iterable | Mapping[str, Any] | None = None,
handler: Callable[[Any], T] | None = None,
split_statements: bool = False,
return_last: bool = True,
) -> T | list[T] | None:
```
Similarly, Leaving as positional args also returns 18 errors.
I believe the only way to avoid Mypy errors are the following 2 options:
1. To do what I did originally, and set `= ...` as the args. Although I
could be wrong, I do not believe this breaks any IDEs or static type checkers.
And Mypy is cool with it.
2. Do what I have in the current PR.
--
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]
