Skip to content

Make the filesystem package independent of the rest of PyAthena #1388

Open

Description

@laughingman7743

Use case

pyathena/filesystem/ imports from the rest of PyAthena, so it cannot be used or released on its own, and changes to the core modules can affect it.
#1363 removed the runtime dependency on Connection and named a possible extraction of the filesystem into its own package as a goal.
This issue makes the dependency one-way: the filesystem imports nothing from PyAthena outside pyathena/filesystem/, and PyAthena depends on the filesystem.
Whether to publish the filesystem as a separate distribution is out of scope and can be decided afterwards.

Current dependencies

After #1378 (and #1386, which removes connection=), the filesystem uses these modules from outside its package:

Dependency Use
pyathena.connection.Connection Type annotation only (TYPE_CHECKING), for connection= and pyathena.filesystem.s3_filesystem()
pyathena.user_agent_extra The botocore user agent of the clients the filesystem builds (pyathena/filesystem/s3.py)
pyathena.util.override Marking overrides
pyathena.util.RetryConfig, retry_api_call, stop_retries_on, unstoppable_retries S3 API retries and the retry stop scopes
pyathena.aio.util.create_in_thread Creating AioS3FileSystem resources in a thread

Proposed change

  1. Move s3_filesystem() to PyAthena's side of the boundary, in a module outside pyathena/filesystem/ that the pandas, Polars and s3fs result sets import.
    It keeps the property agreed in Replace the filesystem's connection= with client= and retry_config= injection #1363: import pyathena does not load fsspec or the filesystem modules.
    Keep pyathena.filesystem.s3_filesystem importable until the next major, or move it in 5.0.0 together with Remove the filesystem's connection= and pass client= to custom s3fs filesystem classes #1386.
  2. Give the filesystem its own user-agent value and override.
  3. Give the filesystem its own retry policy and retry stop scopes, and convert at the boundary.
    • The filesystem owns a retry configuration class (for example S3RetryConfig), its retry function, and its own stop-scope ContextVar with stop_retries_on() and unstoppable_retries().
      It does not need the Athena-specific parts of pyathena.util.retry_api_call, such as unwrapping Glue errors from MetadataException.
    • s3_filesystem() converts connection.retry_config (exceptions, attempt, multiplier, max_delay, exponential_base) into the filesystem's configuration.
    • The cursors and the filesystem currently share one stop scope (_RETRY_STOP_EVENTS in pyathena/util.py).
      For example, the Arrow reader reads files through the filesystem inside stop_retries_on(), so closing the cursor stops the S3 retries.
      To keep that link, pyathena.util.stop_retries_on() and unstoppable_retries() also enter the filesystem's corresponding scopes; the dependency points from PyAthena to the filesystem, as the boundary allows.
      Both variables reach worker threads through contextvars.copy_context() and asyncio.to_thread() as the current one does.
      wait_with_retry_stop() relays only PyAthena's scopes; the filesystem's executor relays its own.
    • The filesystem keeps its own copy of the thread helpers it uses from pyathena/aio/util.py, such as create_in_thread().
    • import pyathena must still not load fsspec once pyathena.util imports the filesystem's retry module.
      pyathena/filesystem/__init__.py imports S3FileSystem and fsspec, so either make its imports lazy or place the retry module where importing it does not run that __init__.py.
      Entering the filesystem's scope only when its module is already loaded is not enough: a filesystem imported inside an open scope would not see the scope.
    • Item 7 of Replace the filesystem's connection= with client= and retry_config= injection #1363 (RetryConfig stays in pyathena.util) still holds; PyAthena's retry module does not move.
  4. Enforce the boundary with a lint rule or a test that fails when a module in pyathena/filesystem/ imports from PyAthena outside the package.
  5. Tests: the filesystem tests create filesystems through s3_filesystem(connect()).
    Decide whether they keep using a connection (acceptable while the code stays in one repository) or build clients from the test environment directly.

Decisions needed before implementing:

  • How import pyathena avoids loading fsspec (item 3): lazy imports in pyathena/filesystem/__init__.py, or the location of the filesystem's retry module.
  • Default error codes of the filesystem's retry policy.
    The default RetryConfig retries ThrottlingException and TooManyRequestsException, which S3 does not return; S3 throttles with SlowDown, which only botocore's own retries handle today.
    A separate policy allows an S3-specific default, which changes the default behavior and needs release notes.
  • Whether this lands in a minor release, or in 5.0.0.

Validation plan (if implementing)

  • just lint, including the boundary check from item 4.
  • The filesystem, Arrow, pandas, Polars, and s3fs tests, including the tests that close a cursor during a read to confirm that the retry stop scopes still stop filesystem retries.
  • import pyathena does not import fsspec (checked through sys.modules).
  • A unit test that a stop scope opened through pyathena.util.stop_retries_on() stops the filesystem's retries, in the same thread and in a thread started with a copy of the context.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions