Skip to content

Migrate Exasol provider to pyexasol 2.x - #69695

Open
joshuabvarghese wants to merge 3 commits into
apache:mainfrom
joshuabvarghese:migrate-exasol-pyexasol2
Open

Migrate Exasol provider to pyexasol 2.x#69695
joshuabvarghese wants to merge 3 commits into
apache:mainfrom
joshuabvarghese:migrate-exasol-pyexasol2

Conversation

@joshuabvarghese

@joshuabvarghese joshuabvarghese commented Jul 10, 2026

Copy link
Copy Markdown

This PR upgrades the Exasol provider to support pyexasol version 2.x.

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.

Key Changes:

  • Variable Separation: Decoupled the Airflow Connection from ExaConnection in get_conn to resolve variable reuse typing errors.
  • Runtime Validation: Added explicit type checks for query_params (enforcing a literal dict to match pyexasol's **query_params unpacking) and sql statements (rejecting lists in get_records/get_first). This respects the DbApiHook base class interface while failing fast on invalid inputs.
  • Dependency Ceiling: Bumped pyproject.toml to pyexasol>=2.0.0,<3, establishing a proper major-version ceiling.
  • Test Coverage: Added negative testing to verify the new runtime validations and fixed a mocked test that was previously hiding a runtime TypeError.
  • Documentation: Updated README.rst, docs/index.rst, and docs/changelog.rst to reflect the breaking changes and new dependencies.

Testing

  • mypy strictness verified locally (0 errors in exasol.py).
  • Unit tests passing.
  • Standalone runtime harness executed against installed pyexasol 2.2.2 to verify end-to-end type execution on conn.execute().

Important

🛠️ Maintainer triage note for @joshuabvarghese · by @potiuk · 2026-07-28 16:32 UTC

Some review feedback from eladkal is waiting on you:

  • There is 1 unresolved review thread on this PR from eladkal.

The ball is in your court — you've been assigned to this PR. Push a fix or reply in each thread explaining why the feedback does not apply, then mark them resolved and ping the reviewer (eladkal) for a final look.

See the Pull Request quality criteria for how to fix each item. There is no rush.

Note: your branch is 294 commits behind main — please rebase and push again to get up-to-date CI results.

Automated triage — may be imperfect; a maintainer takes the next look. We use this two-stage triage process so maintainers' limited time goes to the conversation with you.

@joshuabvarghese
joshuabvarghese marked this pull request as ready for review July 10, 2026 07:28
@potiuk potiuk added the ready for maintainer review Set after triaging when all criteria pass. label Jul 15, 2026
@github-actions

github-actions Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

uv.lock on main just moved via #70932 ("Prepare providers release 2026-08-01"), commit c163931 and this PR currently conflicts.

Quickest fix:

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.

: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):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This rejects any mapping but a dict. This is probably not necessary?

Comment on lines +208 to +212
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."
)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@uranusjr

Copy link
Copy Markdown
Member

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.

@eladkal

eladkal commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

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)
#69695 (comment)
did I miss anything in my earlier comment?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Migrate exasol provider to pyexasol 2.x and remove the <2 cap

5 participants