Skip to content

Align the compliance-suite dialect registry with the entry points - #905

Merged
laughingman7743 merged 1 commit into
masterfrom
fix/test-registry-entry-points
Sep 30, 2026
Merged

laughingman7743 merged 1 commit into
masterfrom
fix/test-registry-entry-points

Conversation

@laughingman7743

@laughingman7743 laughingman7743 commented Sep 30, 2026 •

Copy link
Copy Markdown
Member

WHAT

Make tests/sqlalchemy/__init__.py register the same dialect classes as the sqlalchemy.dialects entry points in pyproject.toml, and add a test that keeps them in sync.

  • Bare awsathena now registers pyathena.sqlalchemy.rest:AthenaRestDialect instead of the base AthenaDialect.
  • awsathena.polars is registered; it was missing.
  • TestAthenaDialect.test_compliance_suite_registry_matches_entry_points runs the file against an empty PluginLoader and compares its registrations with the installed pyathena entry points.

WHY

Follow-up to #839, found by the independent review of its 3.x backport #903.

The file registers the dialects for the SQLAlchemy compliance suite. A registration overrides the installed entry point in any process that imports the file. After #839 pointed the bare awsathena entry point at AthenaRestDialect, this file still registered AthenaDialect. Importing tests.sqlalchemy therefore makes a bare awsathena:// URL create AthenaDialect again, and a single pytest session that collects both tests/pyathena/ and tests/sqlalchemy/ fails #839's test_bare_scheme_uses_rest_driver. CI and the just test recipes run the suites in separate processes, and the compliance suite connects only with +rest / +aiorest, so neither was affected.

The missing awsathena.polars registration had no effect either, because an unregistered name falls back to the entry point. It is added so the file mirrors pyproject.toml and the new test can require an exact match.

The same fix will be cherry-picked into #903 for 3.x, whose copy of the file has the same registrations (it only lacks the license header).

TEST

Tested at d6b9303. Python 3.13.1, SQLAlchemy 2.0.46 (locked). All runs are local and offline; nothing contacted AWS.

  • just format and just lint (license headers, ruff check, ruff format --check, mypy, cfn-lint): clean.
  • With dummy AWS_* variables and --noconftest:
    • pytest tests/pyathena/sqlalchemy/test_base.py::TestAthenaDialect: 41 passed.
    • With master's tests/sqlalchemy/__init__.py, the new test fails and reports both differences: awsathena maps to AthenaDialect instead of AthenaRestDialect, and awsathena.polars is missing.
  • import tests.sqlalchemy followed by create_engine("awsathena://...") creates AthenaRestDialect (it created AthenaDialect before).
  • pytest tests/sqlalchemy/ --collect-only, with and without --dburi async: 1630 tests collected each, the same as master.
  • Not run locally: the AWS compliance suites. This PR changes tests/sqlalchemy/, so marking it Ready runs the PyAthena suite and both SQLAlchemy compliance suites on the newest Python version.

🤖 Generated with Claude Code

tests/sqlalchemy/__init__.py registers the dialects for the SQLAlchemy
compliance suite, and a registration overrides the installed entry point in
any process that imports it. After #839 pointed the bare awsathena entry point
at AthenaRestDialect, the file still registered the base AthenaDialect, and it
never registered awsathena.polars. Register the same classes as pyproject.toml,
and add a test that compares the file's registrations with the installed
entry points.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
from sqlalchemy.dialects import registry

registry.register("awsathena", "pyathena.sqlalchemy.base", "AthenaDialect")
registry.register("awsathena", "pyathena.sqlalchemy.rest", "AthenaRestDialect")

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Self-review round one (implementation behavior). Base 8679f04, head d6b9303.

Covered: both changed files; how PluginLoader.load resolves a name (registered impls first, then auto_fn, then entry points); every pyathena entry point against the file's registrations; test isolation under xdist; the compliance-suite collection.

Result: CLEAN.

  • The file now registers exactly the eleven pyathena entry points; before, awsathena pointed at AthenaDialect and awsathena.polars was absent.
  • The new test swaps sqlalchemy.dialects.registry for an empty PluginLoader through monkeypatch before runpy.run_path, so it sees only this file's registrations and does not depend on which dialects other tests loaded into the global registry. monkeypatch restores the global registry afterwards.
  • The test fails on master's file with both differences and passes after the change. Loading the entry points imports only pyathena dialect modules; the polars dialect modules import polars only under TYPE_CHECKING.
  • pytest tests/sqlalchemy/ --collect-only collects 1630 tests with and without --dburi async, the same as master.

assert bare.url.get_driver_name() == "rest"
assert bare.dialect.dialect_description == "awsathena+rest"

def test_compliance_suite_registry_matches_entry_points(self, monkeypatch):

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Self-review round two (claims, callers, operations). Base 8679f04, head d6b9303.

Claims checked:

  • "A registration overrides the installed entry point": PluginLoader.load returns self.impls[name]() before it consults entry points. Reproduced: importing tests.sqlalchemy on master makes a bare URL create AthenaDialect; with this change it creates AthenaRestDialect.
  • "An unregistered name falls back to the entry point": the same method's last loop, so the missing polars registration had no runtime effect.
  • "CI and the just test recipes run the suites in separate processes, and the compliance suite connects only with +rest / +aiorest": justfile runs tests/pyathena/ and tests/sqlalchemy/ in separate pytest commands; tests/__init__.py builds only +rest / +aiorest URLs.
  • "3.x has the same file content": false as written. 3.x's copy has the same registrations but no license header (Add MIT headers to existing files written solely by the maintainer #805 is not on 3.x). The PR body now says so; the cherry-pick applies to the registration lines only.
  • CI note: the changed paths match the sqla filter (^tests/sqlalchemy/, ^tests/pyathena/(aio/)?sqlalchemy/) and not the spark one, so Ready runs the PyAthena suite without Spark plus both compliance suites.

Callers: only test harness code changes; no library behavior or AWS request path changes.

Result: CLEAN after the PR body correction above.

# registration overrides the installed entry point in that process.
loader = PluginLoader("sqlalchemy.dialects")
monkeypatch.setattr("sqlalchemy.dialects.registry", loader)
runpy.run_path(str(Path(__file__).parents[2] / "sqlalchemy" / "__init__.py"))

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Independent review, relayed. Reviewer: Codex CLI 0.157.1, model gpt-6-sol, reasoning effort high, session 01a0f0e9-ceb7-7911-8021-a2fa694d6043. Static review in a read-only sandbox of a detached snapshot at head d6b9303 (base 8679f04); it received the literal diff without the PR description, commit message, or prior findings, and ran no tests. The snapshot and the PR worktree were unchanged afterwards.

Covered: the two changed files, the pyproject.toml entry points, the compliance suite and its configuration, test commands and CI selection, and AGENTS.md. The snapshot had no .venv, so the reviewer did not read SQLAlchemy's PluginLoader source; round two quotes it.

Result: CLEAN. All 11 registrations match the entry points. The test locates the file from __file__ and restores the registry through monkeypatch, with no working-directory, ordering, or pytest-xdist issue found. The compliance suites use explicit +rest / +aiorest URLs, so the changed bare registration and the added polars registration do not change their dialects.

@laughingman7743
laughingman7743 marked this pull request as ready for review September 30, 2026 06:09
laughingman7743 added a commit that referenced this pull request Sep 30, 2026
…ntry points

tests/sqlalchemy/__init__.py registers the dialects for the SQLAlchemy
compliance suite, and a registration overrides the installed entry point in
any process that imports it. After #839 pointed the bare awsathena entry point
at AthenaRestDialect, the file still registered the base AthenaDialect, and it
never registered awsathena.polars. Register the same classes as pyproject.toml,
and add a test that compares the file's registrations with the installed
entry points.

(cherry picked from commit d6b9303)

Conflict resolution: tests/pyathena/sqlalchemy/test_base.py conflicted because master's import block and the tests around TestAthenaDialect come from PRs that are not on 3.x. Only #905's own lines are applied: the importlib.metadata, runpy, Path, and PluginLoader imports and test_compliance_suite_registry_matches_entry_points, placed after test_bare_scheme_uses_rest_driver as on master. The added lines match #905's commit exactly. tests/sqlalchemy/__init__.py applies unchanged apart from context (3.x's copy has no license header).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@laughingman7743
laughingman7743 merged commit 0a139c4 into master Sep 30, 2026
11 checks passed
@laughingman7743
laughingman7743 deleted the fix/test-registry-entry-points branch September 30, 2026 07:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant