Skip to content

Conversation

@tisnik
Copy link
Contributor

@tisnik tisnik commented Oct 19, 2025

Description

LCORE-741: refactoring quota tables statements

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

  • Documentation

    • Added module-level documentation for the quota management package.
  • Refactor

    • Reorganized quota management infrastructure by consolidating database operations and SQL statements into a dedicated module. This internal restructuring preserves quota initialization, synchronization workflows, and all user-facing quota management functionality, which continue to operate exactly as before without any behavioral changes.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 19, 2025

Walkthrough

The PR refactors quota management SQL statements by extracting them from src/runners/quota_scheduler.py into a new dedicated module src/quota/sql.py, while adding a module docstring to src/quota/__init__.py. The scheduler is updated to import these constants.

Changes

Cohort / File(s) Summary
Quota package initialization
src/quota/__init__.py
Adds module-level docstring "Quota management" to document the package purpose.
SQL constants module
src/quota/sql.py
New module defining five SQL constants: CREATE_QUOTA_TABLE, INCREASE_QUOTA_STATEMENT_PG, INCREASE_QUOTA_STATEMENT_SQLITE, RESET_QUOTA_STATEMENT_PG, and RESET_QUOTA_STATEMENT_SQLITE for quota table operations across database backends.
Quota scheduler refactoring
src/runners/quota_scheduler.py
Removes locally defined SQL constants and imports all five quota SQL statements from src/quota/sql. Preserves all existing function signatures and control flow.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

The refactoring follows a straightforward extraction pattern with no logic changes. Review focuses on verifying SQL constant definitions are syntactically correct and that import statements properly resolve the dependencies in quota_scheduler.py.

Possibly related PRs

  • LCORE-741: quota limiter scheduler #684: Originally introduced the quota scheduler and its inline SQL statements; this PR extracts those constants into a dedicated module for better organization and reusability.

Poem

🐰 SQL strings scattered, now gathered with care,
In a module called quota, organized fair,
Constants extracted, imports align,
DRY principles shine—refactoring divine! ✨

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 title "LCORE-741: refactoring quota tables statements" accurately describes the primary change in this pull request. The changeset extracts SQL quota statements from the quota_scheduler.py module into a new dedicated quota/sql.py module, which is exactly what "refactoring quota tables statements" conveys. The title is concise, clear, and specific—a teammate scanning the history would immediately understand that this PR reorganizes quota-related SQL logic without needing to review the implementation details.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% 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

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/quota/sql.py (1)

3-45: Clean refactoring with correct SQL logic.

The SQL statements are well-structured and correctly handle database-specific syntax differences between PostgreSQL and SQLite. The time-based condition revoked_at < NOW() - INTERVAL ensures quotas are only updated once per period.

Consider adding explicit type annotations to the constants for better IDE support and documentation:

+CREATE_QUOTA_TABLE: str = """
-CREATE_QUOTA_TABLE = """
     CREATE TABLE IF NOT EXISTS quota_limits (
         id              text NOT NULL,
         subject         char(1) NOT NULL,
         quota_limit     int NOT NULL,
         available       int,
         updated_at      timestamp with time zone,
         revoked_at      timestamp with time zone,
         PRIMARY KEY(id, subject)
     );
     """


+INCREASE_QUOTA_STATEMENT_PG: str = """
-INCREASE_QUOTA_STATEMENT_PG = """
     UPDATE quota_limits
        SET available=available+%s, revoked_at=NOW()
      WHERE subject=%s
        AND revoked_at < NOW() - INTERVAL %s ;
     """


+INCREASE_QUOTA_STATEMENT_SQLITE: str = """
-INCREASE_QUOTA_STATEMENT_SQLITE = """
     UPDATE quota_limits
        SET available=available+?, revoked_at=datetime('now')
      WHERE subject=?
        AND revoked_at < datetime('now', ?);
     """


+RESET_QUOTA_STATEMENT_PG: str = """
-RESET_QUOTA_STATEMENT_PG = """
     UPDATE quota_limits
        SET available=%s, revoked_at=NOW()
      WHERE subject=%s
        AND revoked_at < NOW() - INTERVAL %s ;
     """


+RESET_QUOTA_STATEMENT_SQLITE: str = """
-RESET_QUOTA_STATEMENT_SQLITE = """
     UPDATE quota_limits
        SET available=?, revoked_at=datetime('now')
      WHERE subject=?
        AND revoked_at < datetime('now', ?);
     """
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2b1c10f and 828b4ff.

📒 Files selected for processing (3)
  • src/quota/__init__.py (1 hunks)
  • src/quota/sql.py (1 hunks)
  • src/runners/quota_scheduler.py (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

  • src/quota/sql.py
  • src/quota/__init__.py
  • src/runners/quota_scheduler.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/quota/sql.py
  • src/quota/__init__.py
  • src/runners/quota_scheduler.py
**/__init__.py

📄 CodeRabbit inference engine (CLAUDE.md)

Package init.py files contain brief package descriptions

Files:

  • src/quota/__init__.py
🧬 Code graph analysis (1)
src/runners/quota_scheduler.py (1)
src/log.py (1)
  • get_logger (7-13)
🔇 Additional comments (3)
src/quota/__init__.py (1)

1-1: LGTM! Docstring follows guidelines.

The module docstring is concise and clearly describes the package purpose.

src/quota/sql.py (1)

1-1: LGTM! Module docstring is clear and descriptive.

src/runners/quota_scheduler.py (1)

20-26: Excellent refactoring with proper absolute imports.

The SQL statements have been correctly extracted to a dedicated module, and the imports follow the project's absolute import convention.

@tisnik tisnik merged commit 81e2dfc into lightspeed-core:main Oct 19, 2025
18 of 20 checks passed
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.

1 participant