dwreeves commented on code in PR #31846:
URL: https://github.com/apache/airflow/pull/31846#discussion_r1228469022
##########
airflow/providers/exasol/hooks/exasol.py:
##########
@@ -157,15 +161,39 @@ def get_description(statement: ExaStatement) ->
Sequence[Sequence]:
)
return cols
+ @overload
+ def run(
+ self,
+ sql: str | Iterable[str],
+ autocommit: bool = False,
+ parameters: Iterable | Mapping[str, Any] | None = None,
+ handler: None = None,
+ split_statements: bool = False,
+ return_last: bool = True,
+ ) -> None:
+ ...
+
+ @overload
+ def run(
+ self,
+ sql: str | Iterable[str],
+ autocommit: bool = False,
+ parameters: Iterable | Mapping[str, Any] | None = None,
+ handler: Callable[[Any], T] = None, # type: ignore[assignment]
+ split_statements: bool = False,
+ return_last: bool = True,
+ ) -> T | list[T]:
+ ...
+
def run(
self,
sql: str | Iterable[str],
autocommit: bool = False,
- parameters: Iterable | Mapping | None = None,
- handler: Callable | None = None,
+ parameters: Iterable | Mapping[str, Any] | None = None,
Review Comment:
This is the one package where I'm unsure... 🤔 I'm actually thinking, the old
annotation was incorrect and it never allowed for `Iterable` types:
- Note that the original annotation was `Iterable | Mapping | None`. So,
Airflow has said for a while that it allows for an iterable.
- `pyexasol` isn't using a SQLAlchemy connection object, as far as I can
tell.
- When I look at `pyexasol`'s documentation, and I look at the API
https://github.com/exasol/pyexasol/blob/master/docs/REFERENCE.md I don't see
any ability to take in a non-mapping iterable type as a valid input. The reason
the other connections take in an `Iterable` is because they all wrap
SQLAlchemy. But if you look carefully, you'll notice `pyexasol` is completely
foregoing SQLAlchemy.
It's possible the safest thing to do here would be to not touch this file's
`parameters` type annotations at all, and just leave as `Iterable | Mapping |
None` for the `run()` method + don't touch anything else.
Someone who is knowledgeable on `pyexasol` should confirm whether I am
correct that the `Iterable` type should not be here.
--
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]
