Restore library API and fix connection lifecycle#151
Merged
Conversation
- Add Type Annotations - Remove Python2 Support - Fix some "private" methods and attributes that are called across modules
Use the latest pamqp and bump the rabbitpy version
Refactored the `Events` class for improved readability and adherence to Python best practices, including updated docstrings and minor formatting adjustments. Update tests removing conditions for Python 2 support.
The setup_module function and related imports were removed as they are no longer required for test environment configuration. This simplifies the codebase and eliminates unnecessary warning filters and file handling logic.
Refactors the IO module to replace the legacy polling-based IOLoop with an asyncio-based architecture. Introduces tests for the new async IO functionality, ensuring robust handling of socket communication and frame processing.
Replaced outdated state management with asyncio-based logic for cleaner and more consistent socket handling. Added new methods and properties to streamline connection states, error handling, and socket closure. Updated tests accordingly to ensure proper validation of the new behavior.
This commit adds robust error handling for socket operations and adjustments to `_get_ssl_context` to better handle SSL options like `check_hostname`. New tests are included to validate SSL configurations and edge case behaviors. Minor improvements to logging and connection handling ensure better maintainability and debugging.
This update introduces a new `ssl_check_hostname` option to the `url_parser` module. It allows users to enable or disable server hostname validation for SSL connections. Additionally, minor adjustments are made for consistency in SSL-related parameter handling.
Extended the URL parser to include the `capath` parameter, allowing users to specify a directory of CA certificates. This enhances flexibility for configuring SSL/TLS connections.
Streamline error handling by introducing `_add_exception` and refactoring `_on_error`. Improve test coverage with dedicated test cases for edge scenarios, including SSL connections and socket-related errors.
- bootstrap writes .env in KEY=VALUE dotenv format instead of build/test.env with export prefixes; removes redundant wait_for and rabbitmqctl await_startup calls (docker compose up --wait handles it) - Add python-dotenv to dev dependency-group - tests/mixins.py uses dotenv.load_dotenv() instead of manual parsing - tests/integration/test_connection.py reads RABBITMQ_URL from os.environ and skips with @skipUnless when the var is not set (no RabbitMQ running) - CI testing.yaml writes .env directly instead of build/test.env - Add .env to .gitignore Co-Authored-By: Gavin M. Roy <gavinmroy@gmail.com>
Co-Authored-By: Gavin M. Roy <gavinmroy@gmail.com>
Two issues caused test_on_data_received to fail on Python 3.12 where the SSL handshake takes ~800ms vs <200ms on other versions: - asyncSetUp used asyncio.sleep(0.1) to wait for the connection, which was too short. Replaced with a polling loop that yields to the event loop between checks. A blocking events.wait() can't be used because the SSL mock server runs inside the test's event loop — blocking it would deadlock the SSL handshake. - test_on_data_received checked self.io._buffer before getting the frame from the queue, creating a race: the IO thread may not have processed the server's response yet. Reordered so the frame is retrieved first (blocks up to 3s), then the buffer is checked. Co-Authored-By: Gavin M. Roy <gavinmroy@gmail.com>
Address two CodeRabbit issues raised on PR #147: - connection.py: _stop_io() now joins the IO thread after calling stop(), blocking until the thread has exited (up to the configured timeout). This ensures callers of __exit__ and close() can rely on the socket being fully torn down before the state transitions to CLOSED, eliminating the nondeterminism CodeRabbit flagged. - io.py: Introduce a dedicated _stop_requested threading.Event flag. stop() sets this flag in addition to signalling _closed, so a stop() call that arrives while _connect() is still running is not silently discarded when _connect() clears _closed unconditionally (line 208). _connect() now checks _stop_requested before clearing _closed and aborts gracefully if set, preventing the IO thread from entering the read/write loop after an external shutdown request. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Fix connection lifecycle, IO shutdown, and integration test env
Co-Authored-By: Gavin M. Roy <gavinmroy@gmail.com>
Update pamqp dependency to >=4.0,<5.0
Typing infrastructure: - Add rabbitpy/py.typed marker (PEP 561) - Add mypy>=1.20 and basedpyright>=1.38 to dev dependency-group - Add [tool.mypy] strict config with test overrides (disallow_untyped_defs=false, disallow_untyped_calls=false for tests) - Add [tool.basedpyright] standard mode config - Add mypy and basedpyright local pre-commit hooks via uv run --frozen Type annotation work across the library: - url_parser: add SslOptions and ConnectionArgs TypedDicts; parse() returns ConnectionArgs; fix str/bytes ambiguity from urlparse; fix _query_args_multi_key_value return type (str | None not int|float|str) - io: use pamqp.frame.FrameTypes from pamqp 4 as canonical frame union; type all Queue/deque/defaultdict generics; add SslOptions to __init__; add -> None to _run; use local loop variable to avoid ioloop None checks; fix _on_write_ready socket None guard; fix certfile/verify SSL logic - connection: type all Queue generics; use typing.Self for __enter__; -> bool | None on __exit__; args property returns ConnectionArgs; blocked uses bool() to avoid bool | None return - channel0: type pending_frames as Queue[FrameTypes | None] - events: add -> None to __init__ - exceptions: add -> str to all __str__ methods; str(self.args[0]) for ActionException to avoid returning Any - state: -> None on __init__ and _set_state Test fixes: - Use SslOptions TypedDict in all ssl_options args and assignments - Add assert isinstance(...) after assertIsInstance where type checkers need narrowing (frame.locales, context.check_hostname) - Fix bytes/str comparison in MockServer.data_received - Type annotate MockServer protocol methods; BaseTransport for _transport - Add -> None to set_expectation/set_response; type write_frame/header Co-Authored-By: Gavin M. Roy <gavinmroy@gmail.com>
Add py.typed marker, mypy/basedpyright, and full type annotations
Restores all source modules and tests from cd478a2^ that were wiped when the modernization commit staged pre-existing deletions: Source modules restored: - rabbitpy/amqp.py — AMQP adapter class (unopinionated channel API) - rabbitpy/amqp_queue.py — Queue class (was amqp_queue.py; keeps name) - rabbitpy/base.py — AMQPClass, AMQPChannel, StatefulObject base classes - rabbitpy/channel.py — full Channel implementation - rabbitpy/exchange.py — Exchange and typed exchange subclasses - rabbitpy/heartbeat.py — heartbeat thread - rabbitpy/message.py — Message class - rabbitpy/simple.py — one-shot helper functions (consume, get, publish) - rabbitpy/tx.py — Tx transaction class - rabbitpy/utils.py — utilities (urlparse, is_string, parse_qs, unquote, etc.) Removes empty rabbitpy/queue.py placeholder (logic lives in amqp_queue.py). Compatibility fixes for Python 3.11+ and pamqp 4: - Circular imports broken via typing.TYPE_CHECKING guard in base.py - pamqp.specification → pamqp.commands (renamed in pamqp 3+) - message._coerce_properties uses amqp_type() instead of __annotations__ (pamqp 4 uses __slots__, not annotations, for Properties fields) - datetime.datetime.utcnow() → datetime.datetime.now(tz=datetime.UTC) - import mock → from unittest import mock - import unittest2 → import unittest - import urlparse (Py2) → from urllib import parse - unicode() → str() in Python 2 skip-decorated tests - utils.PYTHON3 = True constant restored for skipIf decorators - utils.is_string(), parse_qs(), unquote() restored (tested and used) Tests restored: base_tests, channel_test, helpers, integration_tests, test_amqp, test_exchange, test_message, test_queue, test_tx, utils_tests. mypy/basedpyright overrides added for legacy modules (not yet annotated to strict standards) and legacy test files (intentionally test wrong types). Co-Authored-By: Gavin M. Roy <gavinmroy@gmail.com>
… by CI and CodeRabbit - Apply ruff format and fix 3 auto-fixable lint errors (unused import, unsorted imports) across helpers.py, integration_tests.py, utils_tests.py - Fix invalid type annotation syntax in base.AMQPChannel.write_frame: replace `[base.Frame, heartbeat.Heartbeat]` with proper union syntax - Add missing name type validation in base.AMQPClass.__init__ so test_name_invalid passes (raises ValueError for non-string names) - Fix basic_consume return type annotation to Generator[message.Message, None, None] (it is a generator) - Fix basic_get to return the RPC result instead of discarding it - Remove extra False argument in basic_publish Message constructor call - Add thread-safe _stopped flag to Heartbeat to prevent timer recreation after stop() is called; use lock to guard start/stop/_maybe_send lifecycle - Fix tx.Tx.__exit__ to return False instead of re-raising exc_val, preserving original exception traceback; wrap rollback in try/except - Fix tx.commit/rollback to keep _selected=True on success rather than always clearing it, matching the documented transaction semantics - Fix create_queue in simple.py to skip name validation when queue_name is empty, restoring RabbitMQ auto-naming behaviour Co-Authored-By: Gavin M. Roy <gavinmroy@gmail.com>
TYPE_CHECKING-only imports (chan, conn) were used in class-body annotations evaluated eagerly on Python <3.14. Adding from __future__ import annotations defers annotation evaluation to strings on all supported versions, matching the behavior Python 3.14 provides by default. Co-Authored-By: Gavin M. Roy <gavinmroy@gmail.com>
exchange.py, amqp_queue.py, message.py, and tx.py all imported rabbitpy.channel at module level, but channel.py imports those same modules, creating a cycle. On Python 3.11-3.13 this caused an AttributeError at class definition time because the partially-initialized channel module had no Channel attribute yet. Fix by adding `from __future__ import annotations` (defers all annotation evaluation to runtime) and moving the channel import under `if typing.TYPE_CHECKING:`, matching the pattern already used in base.py. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
channel.py imported amqp and amqp_queue at module level for use in type annotations only, creating a circular import cycle: __init__ -> amqp -> channel -> amqp (circular). Python 3.14 evaluates annotations lazily by default, so the cycle was harmless there, but 3.11/3.12/3.13 evaluate class-body annotations at import time, causing AttributeError on the partially-initialised module. Fix follows the same pattern applied to exchange.py, amqp_queue.py, message.py, and tx.py: add `from __future__ import annotations` to defer all annotation evaluation, and move the amqp/amqp_queue imports under TYPE_CHECKING so they are invisible at runtime. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
The three MockServerConnectedTestCase tests checked _buffer and bytes_received immediately after write_protocol_header() (which only sleeps 100 ms) — before the IO thread had necessarily finished processing the server's response. On Python 3.12 CI runners, asyncio event-loop scheduling latency routinely exceeds 100 ms (300-400 ms observed in logs), so the assertions fired before the IO thread had received or dispatched any data. Fix: move all internal-state assertions (_buffer, bytes_received) to after the blocking queue.get() calls that are already present in each test. Those calls serve as natural synchronisation barriers: they only return once the IO thread has unmarshalled and enqueued the frame, guaranteeing that _buffer and bytes_received are in their final state. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
On Python 3.11 the asyncio writer callback registered on the IO thread's event loop fires more eagerly than on later versions. _on_write_ready is called both directly by the test and concurrently by the writer callback, enqueuing a second ConnectionException into self.exceptions. tearDown treats any remaining exception as an uncaught error and re-raises it. Fix by exiting the mock.patch context before yielding, then draining any duplicate ConnectionExceptions that the IO thread may have added before the write buffer was emptied. Each drained exception is asserted to be a ConnectionException so the drain itself is a test rather than silent suppression. Co-Authored-By: Gavin M. Roy <gavinmroy@gmail.com>
- __init__.py: remove unimported module names (amqp_queue, channel,
connection, exceptions, exchange, message, simple, tx) from __all__
to prevent AttributeError on access
- amqp_queue.py: pass queue=self.name to Queue.Purge() so the correct
queue is purged instead of the AMQP default empty-string queue
- base.py: guard __int__ against None _channel_id with ValueError;
update rpc() and _wait_on_frame() return types to include None to
match their implicit return paths
- exchange.py: remove mutable class-level arguments={} default that
would be shared across instances; fix Exchange.__init__ arguments
annotation from bool|None to dict|None; fix HeadersExchange docstring
typo ("direct" -> "headers")
- message.py: fix json() return type annotation from bytes to Any;
guard maximum_frame_size=0 in publish() with fallback to avoid
ZeroDivisionError
Co-Authored-By: Gavin M. Roy <gavinmroy@gmail.com>
Implements full AMQP 0-9-1 connection negotiation that was missing from the modernized channel0/connection modules, and addresses four open issues: **channel0.py** — full rewrite as a threading.Thread: - Sends ProtocolHeader, handles Connection.Start/Tune/Open handshake - Negotiates channel_max, frame_max, and heartbeat with the server - Propagates negotiation failures to the exceptions queue with proper timeout (#133, #124: prevents indefinite hang when the server comes up mid-retry or returns an error such as an invalid vhost) - Runs a post-open loop to handle Connection.Blocked/Unblocked/Close **connection.py** — completes the implementation: - Adds channel() method, _channels dict, and heartbeat lifecycle - Adds _get_next_channel_id() that reclaims IDs from closed channels before allocating a new one, preventing 65535-channel exhaustion (#121) - connect() waits for CHANNEL0_OPENED with a bounded timeout; surfaces any exception raised by channel0 (invalid vhost, auth failure, etc.) - _close_channels() gracefully closes open channels on connection close **io.py** — adds write-trigger socket pair and shared write queue: - Exposes write_trigger (socket write end) and write_queue so that base.AMQPChannel.write_frame() can enqueue frames and wake the asyncio event loop, maintaining backward compatibility with the restored base.py **state.py** — adds closed/open/opening/closing aliases for compatibility with the restored base.py which uses these names instead of is_closed etc. **base.py** — reduces _read_from_queue() polling timeout from 100 ms to 10 ms; cuts the busy-wait overhead between each published message when publisher confirms are enabled (~10x latency improvement) (#73). Co-Authored-By: Gavin M. Roy <gavinmroy@gmail.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
…nnel0.py - Add full type arguments to queue.Queue annotations in io.py and connection.py (mypy [type-arg] errors) - Fix dict return types on capabilities/server_properties to dict[str, typing.Any] (mypy [type-arg] and [no-any-return] errors) - Apply ruff formatting to channel0.py and connection.py (2 files were flagged as needing reformatting by pre-commit) Co-Authored-By: Gavin M. Roy <gavinr@aweber.com>
…YPE_CHECKING The `from rabbitpy import connection as conn` import caused a circular dependency: __init__ -> amqp -> channel -> connection -> channel. Since `from __future__ import annotations` is already present, the `conn.Connection` type annotation on `Channel.__init__` is evaluated lazily as a string at runtime, so moving the import under TYPE_CHECKING is safe and breaks the cycle. Fixes AttributeError on Python 3.11-3.13 where partially initialized modules do not expose their classes during circular imports. Co-Authored-By: Gavin M. Roy <gavinmroy@gmail.com>
Owner
Author
|
🤖 This comment was posted by Claude on behalf of @gmr PR Monitor SummaryCI FixAll 4 test jobs (Python 3.11, 3.12, 3.13, 3.14) were failing with: ``` The circular import chain was: Fix: Moved `from rabbitpy import connection as conn` in `channel.py` under `TYPE_CHECKING`. This is safe because `channel.py` already has `from future import annotations`, making the `conn.Connection` type annotation lazy (a string at runtime, never evaluated). Commit: 3404ca7 All 199 tests pass locally before pushing. Review ThreadsNo open CodeRabbit review threads found — nothing to resolve. |
pamqp 4.0.0's encode.short_uint requires an int but Connection.Close defaults class_id and method_id to None. Per the AMQP spec, a normal graceful close passes 0 for both fields (no associated method error). Co-Authored-By: Gavin M. Roy <gavinmroy@gmail.com>
This was referenced Apr 1, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Problem
Several modules were deleted in previous cleanup but are still needed as part of the library's public API. Connection lifecycle had issues with proper shutdown ordering.
Solution
Restored deleted modules with proper type annotations. Fixed circular imports using
TYPE_CHECKINGguards. Resolved race conditions in IO tests with proper synchronization.