Skip to content

Issue #42 : [decorators] @cache never evicts; @rate_limit is process-global and raises bare ValueError - #47

Open
Shreyavemana wants to merge 8 commits into
nitrocloudofficial:developfrom
Shreyavemana:feat/cache-eviction-rate-limit
Open

Shreyavemana wants to merge 8 commits into
nitrocloudofficial:developfrom
Shreyavemana:feat/cache-eviction-rate-limit

Conversation

@Shreyavemana

Copy link
Copy Markdown
Collaborator

This PR enhances the decorator utilities in nitrostack/core/additional_decorators.py to make caching and rate limiting safer and more predictable.

Changes

  • @cache
    • Adds max_size parameter (default 1024, None disables cap).
    • Implements expired-first then LRU eviction to prevent unbounded growth.
    • Exposes cache_clear() for tests.
  • @rate_limit
    • Introduces RateLimitExceededError (subclass of ValueError) carrying retry_after_seconds.
    • Supports optional key= callable for per-user or per-resource counters.
    • Error messages now include a “Retry after X seconds” hint.
  • Core
    • Added RateLimitExceededError to nitrostack/core/errors.py and exported in __init__.py.
  • Tests
    • New tests/test_decorators_limits.py covers eviction, expiry, unbounded mode, cache_clear, typed error, retry_after_seconds, per-key limits, and shared default behavior.

Verification

  • New tests plus existing decorator/cache tests pass (30 total).
  • Full suite passes (aside from one unrelated httpx dependency).

Impact

  • Prevents memory leaks from unbounded caches.
  • Provides clearer, typed errors for rate limiting with retry hints.
  • Enables per-user rate limiting strategies.

manish-wekan and others added 8 commits September 10, 2026 18:05
…ase PR.

Keep pyproject.toml and uv.lock in sync, skip a bump while a version is still unpublished, and only hit the PyPI environment when an upload will actually run.
Bring the bump/publish workflows in line with develop (via main) so the PR can merge.
…ure/release-automation

Automate version bumps and PyPI publishes from main
@Shreyavemana

Shreyavemana commented Oct 16, 2026 •

Copy link
Copy Markdown
Collaborator Author

The solution adds an LRU‑bounded cache with a default size of 1024 and a cache_clear() hook, plus a typed RateLimitExceededError that includes retry hints and supports per‑key counters. All decorator tests passed (30 total).

@Shreyavemana
Shreyavemana marked this pull request as ready for review October 16, 2026 10:38
@manish-wekan

Copy link
Copy Markdown
Collaborator

Install the entire nitrostack-py package from output-schema-keep-null branch, either locally or in /tmp/<name_of_dir>/
then import that package
and test it against #42

#42
If the tests pass, add the test report as comments. here.

@Shreyavemana

Copy link
Copy Markdown
Collaborator Author

Verification of the proposed fix (feat/cache-eviction-rate-limit, bfe29e8)

Changes: Improved caching with size limits and a cache-clearing option, along with better rate limiting and per-key limits.

Installation and test environment

Cloned the branch into a temporary directory, created a fresh virtual environment, and installed the package using pip install <repo>.

The checks were run against the installed package from outside the repository, and the import resolved to venv\Lib\site-packages\nitrostack, confirming that the installed package was being tested rather than the source checkout.

Results:

  • 30 tests passed against the installed package, covering decorator limits, caching, and MCP extensions.
  • 822 tests passed in the source checkout.
  • End-to-end verification results:
    -- Installed branch: 9/9 checks passed, covering LRU eviction, cache size limits, expiration, cache_clear(), and rate-limit behavior.
    -- Upstream main (a06624c): 0/9 checks passed, as the tested features are not supported.

Conclusion: The results support the proposed changes to caching and rate limiting, with all reported tests passing in their respective environments.

@manish-wekan manish-wekan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@Shreyavemana
Shreyavemana changed the base branch from main to develop October 23, 2026 14:38

This branch has not been deployed

No deployments
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