Skip to content

LCORE-623: proper gss_encmode config type in PostgreSQL configuration#1761

Merged
tisnik merged 3 commits into
lightspeed-core:mainfrom
tisnik:lcore-623-gss-encmode-config-type
May 19, 2026
Merged

LCORE-623: proper gss_encmode config type in PostgreSQL configuration#1761
tisnik merged 3 commits into
lightspeed-core:mainfrom
tisnik:lcore-623-gss-encmode-config-type

Conversation

@tisnik
Copy link
Copy Markdown
Contributor

@tisnik tisnik commented May 19, 2026

Description

LCORE-623: proper gss_encmode config type in PostgreSQL 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
  • Benchmarks improvement

Tools used to create PR

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

Related Tickets & Documents

  • Related Issue #LCORE-623

Summary by CodeRabbit

  • Documentation

    • Updated API documentation with OpenAPI schema changes for configuration parameters.
  • Bug Fixes

    • Improved PostgreSQL configuration validation to restrict GSS encryption mode to only valid options (disable, prefer, require), preventing invalid configurations from being accepted.
  • Tests

    • Added unit test coverage for PostgreSQL configuration parameter validation.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 19, 2026

Warning

Rate limit exceeded

@tisnik has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 57 minutes and 4 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 3b984a18-3d30-4f57-9d6c-5b8cf24e39dd

📥 Commits

Reviewing files that changed from the base of the PR and between f25a7b4 and b6305dd.

📒 Files selected for processing (1)
  • docs/openapi.json

Walkthrough

The PR tightens the PostgreSQL gss_encmode configuration field to accept only three allowed values: "disable", "prefer", and "require". The Pydantic model field type is narrowed to a Literal, the default constant type is aligned, OpenAPI schema documentation is updated with the enum constraint, and unit tests verify validation behavior for both valid and invalid inputs.

Changes

PostgreSQL GSS encmode constraint

Layer / File(s) Summary
Type constraint and validation
src/models/config.py, src/constants.py, docs/openapi.json, tests/unit/models/config/test_postgresql_database_configuration.py
PostgreSQLDatabaseConfiguration.gss_encmode is narrowed from str to Literal["disable", "prefer", "require"], the default constant type is aligned to Final[Literal["prefer"]], OpenAPI schema documents the enum constraint, and two new unit tests validate that valid values are accepted while invalid values raise ValueError with the expected error message.

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and specifically describes the main change: constraining the gss_encmode configuration type to a Literal with specific allowed values for PostgreSQL configuration.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
✨ Simplify code
  • Create PR with simplified code

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
Copy Markdown
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: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/openapi.json`:
- Around line 16328-16332: The enum change for gss_encmode was made manually and
the OpenAPI file is out of sync; revert the manual edit and regenerate the
OpenAPI schema using the repository's schema generator
(generate_openapi_schema.py invoked via the project's task runner) so the
gss_encmode enum is produced correctly, then commit the regenerated
docs/openapi.json.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 5dde7451-607c-4af2-9d9d-9c6fab715b75

📥 Commits

Reviewing files that changed from the base of the PR and between 559753d and f25a7b4.

📒 Files selected for processing (4)
  • docs/openapi.json
  • src/constants.py
  • src/models/config.py
  • tests/unit/models/config/test_postgresql_database_configuration.py
📜 Review details
⏰ 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). (12)
  • GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
  • GitHub Check: E2E: server mode / ci / group 1
  • GitHub Check: E2E: library mode / ci / group 2
  • GitHub Check: E2E: server mode / ci / group 2
  • GitHub Check: E2E: library mode / ci / group 1
  • GitHub Check: E2E: server mode / ci / group 3
  • GitHub Check: E2E: library mode / ci / group 3
  • GitHub Check: E2E Tests for Lightspeed Evaluation job
  • GitHub Check: Pylinter
  • GitHub Check: build-pr
  • GitHub Check: unit_tests (3.13)
  • GitHub Check: unit_tests (3.12)
🧰 Additional context used
📓 Path-based instructions (4)
src/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.py: Use absolute imports for internal modules: from authentication import get_auth_dependency
Llama Stack imports: Use from llama_stack_client import AsyncLlamaStackClient
Check constants.py for shared constants before defining new ones
All modules must start with descriptive docstrings explaining purpose
Use logger = get_logger(__name__) from log.py for module logging
All functions must have complete type annotations for parameters and return types, use modern syntax (str | int), and include descriptive docstrings
Use snake_case with descriptive, action-oriented names for functions (get_, validate_, check_)
Avoid in-place parameter modification anti-patterns; return new data structures instead of modifying function parameters
Use async def for I/O operations and external API calls
Use standard log levels with clear purposes: debug() for diagnostic info, info() for program execution, warning() for unexpected events, error() for serious problems
All classes must have descriptive docstrings explaining purpose and use PascalCase with standard suffixes: Configuration, Error/Exception, Resolver, Interface
Abstract classes must use ABC with @abstractmethod decorators
Follow Google Python docstring conventions with required sections: Parameters, Returns, Raises, and Attributes for classes

Files:

  • src/models/config.py
  • src/constants.py
src/models/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

Pydantic models must use @model_validator and @field_validator for validation and complete type annotations for all attributes, avoiding Any type

Files:

  • src/models/config.py
src/constants.py

📄 CodeRabbit inference engine (AGENTS.md)

Use constants.py for shared constants with descriptive comments and type hints using Final[type]

Files:

  • src/constants.py
tests/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

tests/**/*.py: Use pytest for all unit and integration tests; do not use unittest
Use pytest.mark.asyncio marker for async tests

Files:

  • tests/unit/models/config/test_postgresql_database_configuration.py
🧠 Learnings (2)
📚 Learning: 2026-01-12T10:58:40.230Z
Learnt from: blublinsky
Repo: lightspeed-core/lightspeed-stack PR: 972
File: src/models/config.py:459-513
Timestamp: 2026-01-12T10:58:40.230Z
Learning: In lightspeed-core/lightspeed-stack, for Python files under src/models, when a user claims a fix is done but the issue persists, verify the current code state before accepting the fix. Steps: review the diff, fetch the latest changes, run relevant tests, reproduce the issue, search the codebase for lingering references to the original problem, confirm the fix is applied and not undone by subsequent commits, and validate with local checks to ensure the issue is resolved.

Applied to files:

  • src/models/config.py
📚 Learning: 2026-02-25T07:46:33.545Z
Learnt from: asimurka
Repo: lightspeed-core/lightspeed-stack PR: 1211
File: src/models/responses.py:8-16
Timestamp: 2026-02-25T07:46:33.545Z
Learning: In the Python codebase, requests.py should use OpenAIResponseInputTool as Tool while responses.py uses OpenAIResponseTool as Tool. This difference is intentional due to differing schemas for input vs output tools in llama-stack-api. Apply this distinction consistently to other models under src/models (e.g., ensure request-related tools use the InputTool variant and response-related tools use the ResponseTool variant). If adding new tools, choose the corresponding InputTool or Tool class based on whether the tool represents input or output, and document the rationale in code comments.

Applied to files:

  • src/models/config.py
🪛 GitHub Actions: OpenAPI (Spectral) / 0_spectral.txt
docs/openapi.json

[error] 1-1: CI check failed: docs/openapi.json is out of date (diff between docs/openapi.json and generated /tmp/openapi-generated.json failed). Regenerate with: uv run scripts/generate_openapi_schema.py docs/openapi.json

🪛 GitHub Actions: OpenAPI (Spectral) / spectral
docs/openapi.json

[error] 1-1: CI check failed: docs/openapi.json is out of date (diff against /tmp/openapi-generated.json). Regenerate with: uv run scripts/generate_openapi_schema.py docs/openapi.json

🔇 Additional comments (4)
src/constants.py (1)

163-163: LGTM!

src/models/config.py (1)

239-244: The Literal values correctly match PostgreSQL's valid gss_encmode parameter options: "disable", "prefer", and "require". The field is properly constrained and the description is accurate.

docs/openapi.json (1)

19258-19264: ⚡ Quick win

Unrelated change in PR scope.

This Solr filter documentation change appears unrelated to the PR objective of fixing PostgreSQL gss_encmode configuration. If this change is intentional, it should be documented in the PR description. If not, regenerating the schema from a clean state should exclude this change.

tests/unit/models/config/test_postgresql_database_configuration.py (1)

269-294: LGTM!

Also applies to: 296-311

Comment thread docs/openapi.json
Comment on lines +16328 to +16332
"enum": [
"disable",
"prefer",
"require"
],
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Regenerate the OpenAPI schema instead of manually editing.

The enum constraint for gss_encmode is correct, but the pipeline failure indicates this file is out of sync and should be regenerated using the script rather than manually edited.

Run the following command to regenerate the schema:

uv run scripts/generate_openapi_schema.py docs/openapi.json
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/openapi.json` around lines 16328 - 16332, The enum change for
gss_encmode was made manually and the OpenAPI file is out of sync; revert the
manual edit and regenerate the OpenAPI schema using the repository's schema
generator (generate_openapi_schema.py invoked via the project's task runner) so
the gss_encmode enum is produced correctly, then commit the regenerated
docs/openapi.json.

@tisnik tisnik merged commit 4498c4f into lightspeed-core:main May 19, 2026
30 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