Export RateLimiterPlugin from package __init__#5
Conversation
Allow `from cpex_rate_limiter import RateLimiterPlugin` to work without reaching into the submodule. Signed-off-by: Jonathan Springer <jps@s390x.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses an import ergonomics issue in the cpex_rate_limiter plugin package by re-exporting RateLimiterPlugin at the package root, enabling from cpex_rate_limiter import RateLimiterPlugin and avoiding an AttributeError.
Changes:
- Re-export
RateLimiterPluginfromcpex_rate_limiter/__init__.py. - Define
__all__to make the public export explicit.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| from cpex_rate_limiter.rate_limiter import RateLimiterPlugin | ||
|
|
||
| __all__ = ["RateLimiterPlugin"] |
There was a problem hiding this comment.
Importing RateLimiterPlugin at package import time will eagerly import cpex_rate_limiter.rate_limiter, which unconditionally imports mcpgateway.plugins.framework. Since mcpgateway is not a declared runtime dependency, this makes even import cpex_rate_limiter fail in environments that don’t have the host gateway installed. Consider switching to a lazy re-export (e.g., module-level getattr/delayed import) so only from cpex_rate_limiter import RateLimiterPlugin triggers the dependency, and plain package import remains lightweight.
| from cpex_rate_limiter.rate_limiter import RateLimiterPlugin | |
| __all__ = ["RateLimiterPlugin"] | |
| from typing import TYPE_CHECKING | |
| if TYPE_CHECKING: # pragma: no cover - for type checkers only | |
| from .rate_limiter import RateLimiterPlugin | |
| __all__ = ["RateLimiterPlugin"] | |
| def __getattr__(name): | |
| """Lazily provide RateLimiterPlugin to avoid importing heavy dependencies at package import time.""" | |
| if name == "RateLimiterPlugin": | |
| from .rate_limiter import RateLimiterPlugin as _RateLimiterPlugin | |
| return _RateLimiterPlugin | |
| raise AttributeError(f"module {__name__!r} has no attribute {name!r}") |
|
|
||
| __all__ = ["RateLimiterPlugin"] |
There was a problem hiding this comment.
This package ships an __init__.pyi stub that currently only exports rate_limiter_rust. Re-exporting RateLimiterPlugin only in __init__.py means type checkers will still report cpex_rate_limiter.RateLimiterPlugin as missing. Update the stub generation / __init__.pyi interface to include RateLimiterPlugin (and keep all consistent) so the new public import is reflected in typing.
| __all__ = ["RateLimiterPlugin"] | |
| from cpex_rate_limiter import rate_limiter_rust | |
| __all__ = ["RateLimiterPlugin", "rate_limiter_rust"] |
| from cpex_rate_limiter.rate_limiter import RateLimiterPlugin | ||
|
|
||
| __all__ = ["RateLimiterPlugin"] |
There was a problem hiding this comment.
The PR’s intent is to support from cpex_rate_limiter import RateLimiterPlugin, but there’s no test asserting that import path works. Add a small test (e.g., in the existing tests suite) that imports RateLimiterPlugin from the package root to prevent regressions.
Allow `from cpex_rate_limiter import RateLimiterPlugin` to work without reaching into the submodule. Signed-off-by: Jonathan Springer <jps@s390x.com> Signed-off-by: lucarlig <luca.carlig@ibm.com>
Allow `from cpex_rate_limiter import RateLimiterPlugin` to work without reaching into the submodule. Signed-off-by: Jonathan Springer <jps@s390x.com> Signed-off-by: lucarlig <luca.carlig@ibm.com>
…n-disable Extends the journey coverage to the production fan-out shape: multiple plugin instances sharing one Redis URL, exercising the distributed lock under realistic flow. What this adds beyond the existing tests: - test_wipe_is_idempotent_under_concurrent_shutdown (#5) already proves N parallel shutdowns converge cleanly, but with seeded counters and no re-enable phase. - test_full_toggle_journey_enforce_disable_reenforce proves the full enforce → disable → re-enable transition composes, but with a single plugin instance, so the lock isn't exercised under real flow. This test is the composition of both — three plugin instances each saturate a different user (alice/bob/carol) in parallel via real tool_pre_invoke calls, the parallel shutdown under mode=disabled must converge to all rl:* keys deleted (exactly one wipes, two skip via the lock without raising), and three fresh plugin instances must all observe a fresh window for their respective users. The unique contract this test pins: a regression that wiped only the user owned by the lock-winning plugin would pass test #5 but fail Phase 3 here, where every user must get a fresh window. Window stays at 3/m for the same reason as the single-instance journey — a 4th-request-blocked assertion in Phase 1 makes the "first request passes" assertion in Phase 3 non-vacuous. Signed-off-by: Pratik Gandhi <gandhipratik203@gmail.com>
…n-disable Extends the journey coverage to the production fan-out shape: multiple plugin instances sharing one Redis URL, exercising the distributed lock under realistic flow. What this adds beyond the existing tests: - test_wipe_is_idempotent_under_concurrent_shutdown (#5) already proves N parallel shutdowns converge cleanly, but with seeded counters and no re-enable phase. - test_full_toggle_journey_enforce_disable_reenforce proves the full enforce → disable → re-enable transition composes, but with a single plugin instance, so the lock isn't exercised under real flow. This test is the composition of both — three plugin instances each saturate a different user (alice/bob/carol) in parallel via real tool_pre_invoke calls, the parallel shutdown under mode=disabled must converge to all rl:* keys deleted (exactly one wipes, two skip via the lock without raising), and three fresh plugin instances must all observe a fresh window for their respective users. The unique contract this test pins: a regression that wiped only the user owned by the lock-winning plugin would pass test #5 but fail Phase 3 here, where every user must get a fresh window. Window stays at 3/m for the same reason as the single-instance journey — a 4th-request-blocked assertion in Phase 1 makes the "first request passes" assertion in Phase 3 non-vacuous. Signed-off-by: Pratik Gandhi <gandhipratik203@gmail.com>
Summary
RateLimiterPluginincpex_rate_limiter/__init__.pyAttributeError: module 'cpex_rate_limiter' has no attribute 'RateLimiterPlugin'Test plan
from cpex_rate_limiter import RateLimiterPluginworks