Skip to content

Conversation

@johnjhughes
Copy link

Summary

  • add an aclose method to the AsyncNetworkBackend interface to make the async backend contract explicit for type checkers

Testing

  • not run (interface-only change)

Copilot AI review requested due to automatic review settings February 3, 2026 15:08
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds an aclose method to the AsyncNetworkBackend interface to make the async backend contract explicit for type checkers.

Changes:

  • Added an aclose method to the AsyncNetworkBackend class that raises NotImplementedError

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +100 to +102
async def aclose(self) -> None:
raise NotImplementedError() # pragma: nocover

Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

Adding this method to the AsyncNetworkBackend interface creates a breaking change for type checkers. None of the existing implementations (AnyIOBackend, TrioBackend, AutoBackend, AsyncMockBackend) have implemented this method, which means:

  1. Type checkers will report that these implementations are missing the required aclose method
  2. Test backends in tests/test_cancellations.py (SlowWriteBackend, SlowReadBackend) will also fail type checking

Additionally, there is no usage of backend.aclose() anywhere in the codebase (e.g., connection pools do not call aclose on their backends), which suggests this method may not be needed yet.

If the goal is to make backends closeable for type checking purposes, all implementations need to be updated in the same PR to include either a working implementation or a no-op pass statement. The AutoBackend implementation in particular would need to delegate to self._backend.aclose() if that backend exists.

Suggested change
async def aclose(self) -> None:
raise NotImplementedError() # pragma: nocover

Copilot uses AI. Check for mistakes.
Comment on lines +100 to +101
async def aclose(self) -> None:
raise NotImplementedError() # pragma: nocover
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

This change creates an asymmetry in the API design. The synchronous NetworkBackend class (line 36) does not have a close method, but now AsyncNetworkBackend has an aclose method. This inconsistency may be confusing for users and library maintainers.

Consider whether:

  1. NetworkBackend should also have a close method for consistency
  2. Or if neither backend type needs lifecycle management methods since they are typically stateless factories

Note that both NetworkStream (line 21) and AsyncNetworkStream (line 66) have close/aclose methods respectively, so the current pattern is that streams (connections) need cleanup but backends (connection factories) do not.

Copilot uses AI. Check for mistakes.
@johnjhughes johnjhughes closed this Feb 3, 2026
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