Skip to content

LCORE-1062: DB connection retry logic#933

Merged
tisnik merged 1 commit intolightspeed-core:mainfrom
tisnik:lcore-1062-db-connection-retry-logic
Dec 17, 2025
Merged

LCORE-1062: DB connection retry logic#933
tisnik merged 1 commit intolightspeed-core:mainfrom
tisnik:lcore-1062-db-connection-retry-logic

Conversation

@tisnik
Copy link
Contributor

@tisnik tisnik commented Dec 17, 2025

Description

LCORE-1062: DB connection retry logic

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

Tools used to create PR

Identify any AI code assistants used in this PR (for transparency and review context)

  • Assisted-by: N/A
  • Generated by: N/A

Related Tickets & Documents

  • Related Issue #LCORE-1062

Summary by CodeRabbit

  • Refactor
    • Database reconnection behavior is now configurable through settings instead of using fixed default values, allowing users to adjust retry attempts and delays based on their environment needs.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 17, 2025

Walkthrough

The PR removes two hardcoded reconnection configuration constants (DATABASE_RECONNECTION_COUNT and RECONNECTION_DELAY) from src/runners/quota_scheduler.py and replaces their usage with configurable values sourced from config.scheduler, making the reconnection behavior externally configurable while preserving existing control flow.

Changes

Cohort / File(s) Summary
Configuration refactoring
src/runners/quota_scheduler.py
Removed hardcoded DATABASE_RECONNECTION_COUNT (10) and RECONNECTION_DELAY (1) constants. Replaced usage with config.scheduler.database_reconnection_count and config.scheduler.database_reconnection_delay respectively. Added clarifying comments for connection attempt logic and fallback behavior.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Verify that config.scheduler exposes both database_reconnection_count and database_reconnection_delay attributes
  • Confirm the constants are removed completely and no other references remain in the codebase
  • Ensure the reconnection logic behavior is identical to the previous implementation

Possibly related PRs

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title references a ticket (LCORE-1062) but lacks specificity about what the actual change accomplishes. The phrase 'DB connection retry logic' is vague and doesn't clearly convey that this replaces hardcoded constants with configurable values. Consider a more specific title such as 'LCORE-1062: Make DB reconnection parameters configurable' to clearly describe the main change.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
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

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ade5b5d and 6e69896.

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

📄 CodeRabbit inference engine (CLAUDE.md)

src/**/*.py: Use absolute imports for internal modules in LCS project (e.g., from auth import get_auth_dependency)
All modules must start with descriptive docstrings explaining their purpose
Use logger = logging.getLogger(__name__) pattern for module logging
All functions must include complete type annotations for parameters and return types, using modern syntax (str | int) and Optional[Type] or Type | None
All functions must have docstrings with brief descriptions following Google Python docstring conventions
Function names must use snake_case with descriptive, action-oriented names (get_, validate_, check_)
Avoid in-place parameter modification anti-patterns; return new data structures instead of modifying input parameters
Use async def for I/O operations and external API calls
All classes must include descriptive docstrings explaining their purpose following Google Python docstring conventions
Class names must use PascalCase with descriptive names and standard suffixes: Configuration for config classes, Error/Exception for exceptions, Resolver for strategy patterns, Interface for abstract base classes
Abstract classes must use ABC with @abstractmethod decorators
Include complete type annotations for all class attributes in Python classes
Use import logging and module logger pattern with standard log levels: debug, info, warning, error

Files:

  • src/runners/quota_scheduler.py
⏰ 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). (8)
  • GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
  • GitHub Check: build-pr
  • GitHub Check: E2E: server mode / ci
  • GitHub Check: E2E: library mode / vertexai
  • GitHub Check: E2E: library mode / ci
  • GitHub Check: E2E: server mode / azure
  • GitHub Check: E2E: server mode / vertexai
  • GitHub Check: E2E: library mode / azure
🔇 Additional comments (1)
src/runners/quota_scheduler.py (1)

59-73: Configuration attributes are already validated; suggested validation is unnecessary.

The database_reconnection_count and database_reconnection_delay fields in QuotaSchedulerConfiguration are defined using Pydantic's PositiveInt, which automatically enforces that values must be greater than 0. This validation occurs at model instantiation, preventing the edge cases you mentioned (zero values and negative numbers). The test suite confirms this behavior with tests that verify ValidationError is raised for invalid values. No additional validation in the retry logic is needed.

Likely an incorrect or invalid review 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.

@tisnik tisnik merged commit 2b49a58 into lightspeed-core:main Dec 17, 2025
19 of 27 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