LCORE-741: quota limiters in configuration

[tisnik]

Description

LCORE-741: quota limiters in configuration

Type of change

• Refactor
• New feature
• Bug fix
• CVE fix
• Optimization
• Documentation Update
• Configuration Update
• Bump-up service version
• Bump-up dependent library
• Bump-up library or tool used for development (does not change the final image)
• CI configuration change
• Konflux configuration change
• Unit tests improvement
• Integration tests improvement
• End to end tests improvement

Related Tickets & Documents

• Related Issue #LCORE-741

Summary by CodeRabbit

• New Features

  • Integrated quota handling into application configuration
  • Exposed quota limiters property with lazy initialization
  • Added configuration accessor for quota-related settings

• Bug Fixes

  • Accessing quota settings now raises a clear error when configuration isn't loaded
  • Lazy initialization reuses cached quota limiters after first access

• Tests

  • Added tests for quota handlers loading, limiter initialization, and scheduler behavior
  • Added tests for error handling when configuration is unavailable

[coderabbitai]

Walkthrough

Adds quota-handling accessors to AppConfig: introduces a cached private _quota_limiters, a quota_handlers_configuration property, and a lazily-initialized quota_limiters property that builds limiters via QuotaLimiterFactory; both accessors raise LogicError if configuration is not loaded.

Changes

Cohort / File(s)	Change Summary
Configuration: quota integration src/configuration.py	Added imports for QuotaLimiter and QuotaLimiterFactory. Added private _quota_limiters: list[QuotaLimiter]. Added quota_handlers_configuration property that returns quota handlers config (raises LogicError if config not loaded). Added quota_limiters property that lazily builds and caches limiters via QuotaLimiterFactory.quota_limiters.
Unit tests: configuration quota coverage tests/unit/test_configuration.py	Reset _quota_limiters in test setup. Added assertions that accessing quota properties before load raises LogicError. Added tests loading YAML with quota_handlers (with and without storage) asserting storage fields, limiter counts and string representations, and scheduler period.

Sequence Diagram(s)

sequenceDiagram
    participant Caller as Caller
    participant AppConfig as AppConfig
    participant Factory as QuotaLimiterFactory

    Caller->>AppConfig: access quota_limiters
    alt first access (_quota_limiters is None)
        AppConfig->>AppConfig: ensure configuration loaded (or raise LogicError)
        AppConfig->>Factory: QuotaLimiterFactory.quota_limiters(quota_handlers_config)
        Factory-->>AppConfig: list[QuotaLimiter]
        AppConfig->>AppConfig: cache in _quota_limiters
        AppConfig-->>Caller: return list[QuotaLimiter]
    else subsequent access (cached)
        AppConfig-->>Caller: return cached _quota_limiters
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Verify LogicError paths when configuration is not loaded.
• Check lazy initialization and caching correctness (consider concurrency implications).
• Validate tests' expected limiter string formats and YAML parsing.

Possibly related PRs

• LCORE-741: quota handlers config #676 — Adds the quota handlers configuration model and Configuration.quota_handlers field consumed here.
• LCORE-741: quota limiter implementation for PostgreSQL #701 — Implements limiter types and expected string formats referenced by the new tests.
• LCORE-739: quota limiter stub code #692 — Introduces QuotaLimiter and QuotaLimiterFactory used by the new property.

Poem

🐇 I nibble configs late at night,
I stitch limiters soft and light,
The Factory hums, the cache holds fast,
Quotas wake from YAML past —
Hop, build, and guard each byte.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The PR title "quota limiters in configuration" directly describes the primary change in the changeset. The modifications add quota handling integration to the configuration system by introducing new properties (quota_limiters and quota_handlers_configuration) and lazy initialization of quota limiters. The title is concise, specific, and uses clear terminology rather than vague language. A teammate reviewing git history would immediately understand that this PR implements quota limiter configuration functionality, which aligns with the actual changes to both the configuration module and its corresponding tests.
Docstring Coverage	✅ Passed	Docstring coverage is 92.31% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 3

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 22c3c64 and 24d3a9c.

📒 Files selected for processing (2)

• src/configuration.py (4 hunks)
• tests/unit/test_configuration.py (2 hunks)

🧰 Additional context used

📓 Path-based instructions (6)

src/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Use absolute imports for internal modules (e.g., from auth import get_auth_dependency)

Files:

• src/configuration.py

**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.py: All modules start with descriptive module-level docstrings explaining purpose
Use logger = logging.getLogger(name) for module logging after import logging
Define type aliases at module level for clarity
All functions require docstrings with brief descriptions
Provide complete type annotations for all function parameters and return types
Use typing_extensions.Self in model validators where appropriate
Use modern union syntax (str | int) and Optional[T] or T | None consistently
Function names use snake_case with descriptive, action-oriented prefixes (get_, validate_, check_)
Avoid in-place parameter modification; return new data structures instead of mutating arguments
Use appropriate logging levels: debug, info, warning, error with clear messages
All classes require descriptive docstrings explaining purpose
Class names use PascalCase with conventional suffixes (Configuration, Error/Exception, Resolver, Interface)
Abstract base classes should use abc.ABC and @AbstractMethod for interfaces
Provide complete type annotations for all class attributes
Follow Google Python docstring style for modules, classes, and functions, including Args, Returns, Raises, Attributes sections as needed

Files:

• src/configuration.py
• tests/unit/test_configuration.py

src/{models/config.py,configuration.py}

📄 CodeRabbit inference engine (CLAUDE.md)

src/{models/config.py,configuration.py}: All configuration uses Pydantic models extending ConfigurationBase
Configuration base models must set model_config with extra="forbid" to reject unknown fields

Files:

• src/configuration.py

src/{models/**/*.py,configuration.py}

📄 CodeRabbit inference engine (CLAUDE.md)

src/{models/**/*.py,configuration.py}: Use @field_validator and @model_validator for custom validation in Pydantic models
Use precise type hints in configuration (e.g., Optional[FilePath], PositiveInt, SecretStr)

Files:

• src/configuration.py

tests/{unit,integration}/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

tests/{unit,integration}/**/*.py: Use pytest for all unit and integration tests
Do not use unittest in tests; pytest is the standard

Files:

• tests/unit/test_configuration.py

tests/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

tests/**/*.py: Use pytest-mock to create AsyncMock objects for async interactions in tests
Use the shared auth mock constant: MOCK_AUTH = ("mock_user_id", "mock_username", False, "mock_token") in tests

Files:

• tests/unit/test_configuration.py

🧬 Code graph analysis (2)

src/configuration.py (3)

src/models/config.py (1)

• QuotaHandlersConfiguration (584-593)

src/quota/quota_limiter.py (1)

• QuotaLimiter (19-93)

src/quota/quota_limiter_factory.py (1)

• QuotaLimiterFactory (17-66)

tests/unit/test_configuration.py (1)

src/configuration.py (5)

• quota_handlers_configuration (151-155)
• conversation_cache (158-166)
• quota_limiters (169-177)
• AppConfig (39-177)
• load_configuration (56-62)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)

• GitHub Check: build-pr
• GitHub Check: e2e_tests (azure)
• GitHub Check: e2e_tests (ci)

🔇 Additional comments (5)

tests/unit/test_configuration.py (3)

81-83: LGTM! Consistent error handling test.

The test correctly verifies that accessing quota_handlers_configuration before loading configuration raises the expected error, following the same pattern as other configuration property tests.

---

89-91: LGTM! Consistent error handling test.

The test correctly verifies that accessing quota_limiters before loading configuration raises the expected error.

---

742-806: LGTM! Comprehensive quota handlers test.

The test thoroughly verifies quota handlers configuration with storage, including:

• Storage configuration
• Number of limiters created
• Specific string representations of limiters
• Scheduler configuration

The string representation assertions provide good validation of the actual limiter instances created by the factory.

src/configuration.py (2)

23-23: LGTM! Imports follow conventions.

The new imports for quota handling functionality follow the existing import patterns and coding guidelines for absolute imports.

Also applies to: 29-30

---

150-155: LGTM! Consistent configuration accessor.

The quota_handlers_configuration property correctly follows the established pattern for configuration accessors, including proper error handling when configuration is not loaded.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

src/configuration.py (1)

172-181: LGTM with optional improvement suggestion.

The lazy initialization pattern correctly mirrors conversation_cache and properly caches the quota limiters after first access. The implementation is functionally correct.

Optional improvement: When QuotaLimiterFactory.quota_limiters() legitimately returns an empty list (e.g., no storage configured), the condition if not self._quota_limiters: will remain True on every access, causing repeated factory calls. Consider using a sentinel pattern:

 def __init__(self) -> None:
     """Initialize the class instance."""
     self._configuration: Optional[Configuration] = None
     self._conversation_cache: Optional[Cache] = None
-    self._quota_limiters: list[QuotaLimiter] = []
+    self._quota_limiters: Optional[list[QuotaLimiter]] = None

 def init_from_dict(self, config_dict: dict[Any, Any]) -> None:
     """Initialize configuration from a dictionary."""
     # clear cached values when configuration changes
     self._conversation_cache = None
-    self._quota_limiters = []
+    self._quota_limiters = None
     # now it is possible to re-read configuration
     self._configuration = Configuration(**config_dict)

 @property
 def quota_limiters(self) -> list[QuotaLimiter]:
     """Return list of all setup quota limiters."""
     if self._configuration is None:
         raise LogicError("logic error: configuration is not loaded")
-    if not self._quota_limiters:
+    if self._quota_limiters is None:
         self._quota_limiters = QuotaLimiterFactory.quota_limiters(
             self._configuration.quota_handlers
         )
     return self._quota_limiters

This avoids repeated factory invocation when the factory returns an empty list, though the current implementation is acceptable given minimal impact.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 24d3a9c and b312f60.

📒 Files selected for processing (2)

• src/configuration.py (5 hunks)
• tests/unit/test_configuration.py (3 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• tests/unit/test_configuration.py

🧰 Additional context used

📓 Path-based instructions (4)

src/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Use absolute imports for internal modules (e.g., from auth import get_auth_dependency)

Files:

• src/configuration.py

**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.py: All modules start with descriptive module-level docstrings explaining purpose
Use logger = logging.getLogger(name) for module logging after import logging
Define type aliases at module level for clarity
All functions require docstrings with brief descriptions
Provide complete type annotations for all function parameters and return types
Use typing_extensions.Self in model validators where appropriate
Use modern union syntax (str | int) and Optional[T] or T | None consistently
Function names use snake_case with descriptive, action-oriented prefixes (get_, validate_, check_)
Avoid in-place parameter modification; return new data structures instead of mutating arguments
Use appropriate logging levels: debug, info, warning, error with clear messages
All classes require descriptive docstrings explaining purpose
Class names use PascalCase with conventional suffixes (Configuration, Error/Exception, Resolver, Interface)
Abstract base classes should use abc.ABC and @AbstractMethod for interfaces
Provide complete type annotations for all class attributes
Follow Google Python docstring style for modules, classes, and functions, including Args, Returns, Raises, Attributes sections as needed

Files:

• src/configuration.py

src/{models/config.py,configuration.py}

📄 CodeRabbit inference engine (CLAUDE.md)

src/{models/config.py,configuration.py}: All configuration uses Pydantic models extending ConfigurationBase
Configuration base models must set model_config with extra="forbid" to reject unknown fields

Files:

• src/configuration.py

src/{models/**/*.py,configuration.py}

📄 CodeRabbit inference engine (CLAUDE.md)

src/{models/**/*.py,configuration.py}: Use @field_validator and @model_validator for custom validation in Pydantic models
Use precise type hints in configuration (e.g., Optional[FilePath], PositiveInt, SecretStr)

Files:

• src/configuration.py

🧬 Code graph analysis (1)

src/configuration.py (4)

src/models/config.py (1)

• QuotaHandlersConfiguration (584-593)

src/cache/cache_factory.py (1)

• CacheFactory (16-49)

src/quota/quota_limiter.py (1)

• QuotaLimiter (19-93)

src/quota/quota_limiter_factory.py (1)

• QuotaLimiterFactory (17-66)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)

• GitHub Check: build-pr
• GitHub Check: e2e_tests (ci)
• GitHub Check: e2e_tests (azure)

🔇 Additional comments (4)

src/configuration.py (4)

23-23: LGTM! Imports follow guidelines.

The quota handling imports are necessary for the new functionality and follow the absolute import pattern as per coding guidelines.

Also applies to: 29-30

---

54-54: LGTM! Proper initialization with type annotation.

The _quota_limiters attribute is correctly initialized with a complete type annotation that matches the property return type.

---

64-70: Excellent fix! Cache invalidation now properly implemented.

The cache clearing for both _conversation_cache and _quota_limiters before reloading configuration correctly addresses the critical cache invalidation issue flagged in previous reviews. This ensures quota limiters and conversation cache reflect the newly loaded configuration rather than stale values.

---

154-159: LGTM! Property follows established patterns.

The quota_handlers_configuration property correctly follows the same pattern as other configuration accessors, with consistent error handling and proper type annotations.