Skip to content

[5.28] Cache sockname and peername#1313

Open
robsdedude wants to merge 3 commits into
5.0from
fix/own-sockname-caching
Open

[5.28] Cache sockname and peername#1313
robsdedude wants to merge 3 commits into
5.0from
fix/own-sockname-caching

Conversation

@robsdedude
Copy link
Copy Markdown
Member

@robsdedude robsdedude commented Jun 5, 2026

Async

Instead of relying on asyncio's caching of these properties, we do it ourselves. The advantage is that asyncio populates the cache on a best-effort basis. This can lead to the values being None if retrieving them causes an OSError.

This is a misalignment between the async and sync driver that this PR aims to remedy.

Sync

While in the async driver we're introducing (custom) caching where implicit caching was already in place, we also introduce caching of these fields in the sync driver for parity.

Fixes: #1310
Part of: DRIVERS-418

Async
-----
Instead of relying on asyncio's caching of these properties, we do it ourselves.
The advantage is that asyncio populates the cache on a best-effort basis. This
can lead to the values being `None` if retrieving them causes an `OSError`.

This is a misalignment between the async and sync driver that this PR aims to
remedy.

Sync
----
While in then async driver we're introducing (custom) caching where implicit
caching was already in place, we also introduce caching of these fields in the
sync driver for parity.
@robsdedude robsdedude force-pushed the fix/own-sockname-caching branch from b60baef to f1ec1fa Compare June 5, 2026 13:19
@robsdedude robsdedude marked this pull request as ready for review June 8, 2026 09:03
Copy link
Copy Markdown
Contributor

@AndyHeap-NeoTech AndyHeap-NeoTech left a comment

Choose a reason for hiding this comment

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

Looks good, just one small comment for clarification.

Comment thread src/neo4j/_async_compat/network/_bolt_socket.py Outdated
Copy link
Copy Markdown
Contributor

@AndyHeap-NeoTech AndyHeap-NeoTech left a comment

Choose a reason for hiding this comment

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

LGTM.

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.

2 participants