You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "joshuabvarghese (via GitHub)" <gi...@apache.org> on 2026/07/10 07:22:22 UTC
[PR] Migrate Exasol provider to pyexasol 2.x [airflow]
joshuabvarghese opened a new pull request, #69695:
URL: https://github.com/apache/airflow/pull/69695
This PR upgrades the Exasol provider to support `pyexasol` version 2.x.
* closes: #12345
As `pyexasol` 2.x introduces changes to the underlying library API, this PR includes the necessary refactoring of the Exasol hook, updates to the unit tests, and adjustments to the provider's dependencies and documentation to maintain full compatibility.
### Changes:
- Updated `pyproject.toml` to support `pyexasol>=2.0.0`
- Updated `providers/exasol/src/airflow/providers/exasol/hooks/exasol.py` to align with the 2.x API.
- Updated unit tests in `providers/exasol/tests/unit/exasol/hooks/test_exasol.py`.
- Updated `providers/exasol/README.rst`, `docs/index.rst`, and `docs/changelog.rst`.
- Updated `uv.lock`.
### Testing:
- Verified that all unit tests for the Exasol provider pass successfully.
- Verified local builds for the provider package.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Migrate Exasol provider to pyexasol 2.x [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on PR #69695:
URL: https://github.com/apache/airflow/pull/69695#issuecomment-5116666207
It seems like the upgrade is quite breaking (a lot of old inputs are not rejected), so the provider will need a major version bump. If we’re bumping major, deprecated functions should also be removed.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Migrate Exasol provider to pyexasol 2.x [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #69695:
URL: https://github.com/apache/airflow/pull/69695#discussion_r3673327610
##########
providers/exasol/src/airflow/providers/exasol/hooks/exasol.py:
##########
@@ -67,21 +67,36 @@ def __init__(self, *args, sqlalchemy_scheme: str | None = None, **kwargs) -> Non
self.schema = kwargs.pop("schema", None)
self._sqlalchemy_scheme = sqlalchemy_scheme
+ @staticmethod
+ def _validate_query_params(parameters: Iterable | Mapping[str, Any] | None) -> dict[str, Any] | None:
+ """
+ Validate that query parameters are a dict, as required by pyexasol.
+
+ :param parameters: The parameters as passed to this hook's public methods.
+ :return: The same parameters, narrowed to ``dict[str, Any] | None`` for pyexasol/mypy.
+ """
+ if parameters is not None and not isinstance(parameters, dict):
Review Comment:
This rejects any mapping _but_ a dict. This is probably not necessary?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Migrate Exasol provider to pyexasol 2.x [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on code in PR #69695:
URL: https://github.com/apache/airflow/pull/69695#discussion_r3597810778
##########
providers/exasol/docs/changelog.rst:
##########
@@ -27,6 +27,26 @@
Changelog
---------
+Breaking changes
+~~~~~~~~~~~~~~~~
+
+* ``Migrate exasol provider to pyexasol 2.x; remove the <2 cap (#69123)``
+
+ This provider now requires ``pyexasol>=2.0.0,<3``. pyexasol 2.0.0 ships a ``py.typed`` marker
+ (`pyexasol changelog <https://exasol.github.io/pyexasol/master/changes/changes_2.html>`__), which
+ surfaces several previously-hidden type mismatches in the Exasol hook:
+
+ * ``ExasolHook.get_records()`` and ``ExasolHook.get_first()`` now only accept a single SQL string,
+ not a list of statements (pyexasol's underlying ``execute()`` never actually supported a list; use
+ ``ExasolHook.run()`` for executing multiple statements in sequence).
+ * ``parameters``/``query_params`` passed to ``get_records()``, ``get_first()``, ``run()``, and
+ ``get_df()``/``get_pandas_df()`` must be a ``dict``/``Mapping``. Positional-style parameters
+ (lists/tuples) were never actually supported by pyexasol at runtime and will now raise a clear
+ ``TypeError`` instead of failing deep inside pyexasol (or silently doing the wrong thing).
+ * If you pass ``export_params={"with_column_names": ...}`` to ``ExasolHook.export_to_file()`` or
+ ``ExasolToS3Operator``, note that pyexasol 2.0.0 fixed this option to be interpreted as a real
+ boolean; previously any value (including ``False``) was treated as ``True``.
Review Comment:
The list note stuff but nothing here strike as airflow breaking change is it?
Can you just remove the breaking changes title?
also, did you validate these statements? This feels AI generated. Please provide evidences that the points here are valid. Just referencing to the upstream docs/code should be enough
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Migrate Exasol provider to pyexasol 2.x [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on PR #69695:
URL: https://github.com/apache/airflow/pull/69695#issuecomment-5462869280
Fixed in https://github.com/apache/airflow/pull/72119
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Migrate Exasol provider to pyexasol 2.x [airflow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #69695:
URL: https://github.com/apache/airflow/pull/69695#issuecomment-4983499944
<!-- uv-lock-rebase-notice -->
`uv.lock` on `main` just moved via [#69924](https://github.com/apache/airflow/pull/69924) ("Support excluding files from `common.ai` Agent Skills discovery"), commit [`663a792`](https://github.com/apache/airflow/commit/663a79203891e04111e4a0cea079cc34011a9ad9) and this PR currently conflicts.
Quickest fix:
```bash
git fetch upstream main && git rebase upstream/main
rm uv.lock && uv lock
git add uv.lock && git rebase --continue
git push --force-with-lease
```
_Automated nudge — ignore if you're not ready to rebase. This comment is updated in place on future `uv.lock` bumps._
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Migrate Exasol provider to pyexasol 2.x [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal closed pull request #69695: Migrate Exasol provider to pyexasol 2.x
URL: https://github.com/apache/airflow/pull/69695
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Migrate Exasol provider to pyexasol 2.x [airflow]
Posted by "baha-bouali (via GitHub)" <gi...@apache.org>.
baha-bouali commented on code in PR #69695:
URL: https://github.com/apache/airflow/pull/69695#discussion_r3651405229
##########
providers/exasol/docs/changelog.rst:
##########
@@ -27,6 +27,26 @@
Changelog
---------
+Breaking changes
+~~~~~~~~~~~~~~~~
+
+* ``Migrate exasol provider to pyexasol 2.x; remove the <2 cap (#69123)``
+
+ This provider now requires ``pyexasol>=2.0.0,<3``. pyexasol 2.0.0 ships a ``py.typed`` marker
+ (`pyexasol changelog <https://exasol.github.io/pyexasol/master/changes/changes_2.html>`__), which
+ surfaces several previously-hidden type mismatches in the Exasol hook:
+
+ * ``ExasolHook.get_records()`` and ``ExasolHook.get_first()`` now only accept a single SQL string,
+ not a list of statements (pyexasol's underlying ``execute()`` never actually supported a list; use
+ ``ExasolHook.run()`` for executing multiple statements in sequence).
+ * ``parameters``/``query_params`` passed to ``get_records()``, ``get_first()``, ``run()``, and
+ ``get_df()``/``get_pandas_df()`` must be a ``dict``/``Mapping``. Positional-style parameters
+ (lists/tuples) were never actually supported by pyexasol at runtime and will now raise a clear
+ ``TypeError`` instead of failing deep inside pyexasol (or silently doing the wrong thing).
+ * If you pass ``export_params={"with_column_names": ...}`` to ``ExasolHook.export_to_file()`` or
+ ``ExasolToS3Operator``, note that pyexasol 2.0.0 fixed this option to be interpreted as a real
+ boolean; previously any value (including ``False``) was treated as ``True``.
Review Comment:
There are two things I can confirm with the changelog
* `with_column_names` boolean fix: I can confirm that pyexasol's changelog has a dedicated bugfix (https://github.com/exasol/pyexasol/releases#release-2.2.1 - \# 265)). So any value used to get forced to be `True`. Now it should require a real Boolean
* `py.typed` marker: Confirmed. In the Same Changelog batch above (# 298) pyexasol added it when swapping the deprecated `rsa` dependency for `cryptography`. I think that this is the actual mechanism not a breaking change.
However,
* `get_records()`/`get_first()` only accepting a single SQL string. I cannot confirm this one tho. No changelog says explicitly this changed in 2.x. However, it's consistent with every example in pyexasol's docs since they only show a single string. This means that's always the case.
* Parameters should be `dict/Mapping`: Can't confirm either, but no changelog ref to point to.
So, two claims are true, two are unverified assumptions.
I suggest to drop the "*Breaking changes*" title since it is not a change to the provider's own interface and drop the two unverifiable bullets entirely. Keep only the two we can back:
```
This provider now requires ``pyexasol>=2.0.0,<3``. pyexasol 2.0.0 added a
``py.typed`` marker and fixed ``export_params={"with_column_names": ...}``
to require a real boolean (previously any value, including ``False``, was
treated as ``True`` — pyexasol changelog, bugfix #265).
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Migrate Exasol provider to pyexasol 2.x [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #69695:
URL: https://github.com/apache/airflow/pull/69695#discussion_r3673386610
##########
providers/exasol/src/airflow/providers/exasol/hooks/exasol.py:
##########
@@ -184,11 +199,19 @@ def get_records(
"""
Execute the SQL and return a set of records.
- :param sql: the sql statement to be executed (str) or a list of
- sql statements to execute
- :param parameters: The parameters to render the SQL query with.
+ :param sql: the sql statement to be executed. Must be a single ``str``; pyexasol's ``execute()``
+ does not accept a list of statements. For executing multiple statements in sequence, use
+ :meth:`run` with ``handler=exasol_fetch_all_handler`` instead.
+ :param parameters: The parameters to render the SQL query with. Must be a ``dict``/``Mapping``;
+ pyexasol does not support positional (list/tuple) query parameters.
"""
- with closing(self.get_conn()) as conn, closing(conn.execute(sql, parameters)) as cur:
+ if not isinstance(sql, str):
+ raise TypeError(
+ "ExasolHook.get_records() only supports a single SQL string, not a list of statements. "
+ "Use ExasolHook.run() with a handler for executing multiple statements."
+ )
Review Comment:
This is duplicated too many times. I would refactor it into a private function called by each place. You can use `TypeGuard` to improve typing.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Migrate Exasol provider to pyexasol 2.x [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on PR #69695:
URL: https://github.com/apache/airflow/pull/69695#issuecomment-5117410361
> It seems like the upgrade is quite breaking (a lot of old inputs are not rejected), so the provider will need a major version bump. If we’re bumping major, deprecated functions should also be removed.
The breaking changes don't seem to be about Airflow interface (Hook/operator)
https://github.com/apache/airflow/pull/69695#discussion_r3597810778
did I miss anything in my earlier comment?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org