-
Notifications
You must be signed in to change notification settings - Fork 2
Guardrails: Config Management #30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
a537bf0
4174e90
f691bcb
cfe8a84
3a0fa81
0d2d609
d445ad9
59deffe
97dae6c
29431e0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| """Added validator_config table | ||
|
|
||
| Revision ID: 003 | ||
| Revises: 002 | ||
| Create Date: 2026-02-05 09:42:54.128852 | ||
|
|
||
| """ | ||
| from typing import Sequence, Union | ||
|
|
||
| from alembic import op | ||
| from sqlalchemy.dialects import postgresql | ||
| import sqlalchemy as sa | ||
|
|
||
| # revision identifiers, used by Alembic. | ||
| revision: str = '003' | ||
| down_revision: str = '002' | ||
| branch_labels: Union[str, Sequence[str], None] = None | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what are these variables for , "branch labels" and "depends_on", if they are not being used in any way why are they there |
||
| depends_on: Union[str, Sequence[str], None] = None | ||
|
|
||
|
|
||
| def upgrade() -> None: | ||
| op.create_table('validator_config', | ||
| sa.Column('id', sa.Uuid(), nullable=False), | ||
| sa.Column('organization_id', sa.Integer(), nullable=False), | ||
| sa.Column('project_id', sa.Integer(), nullable=False), | ||
| sa.Column('type', sa.String(), nullable=False), | ||
| sa.Column('stage', sa.String(), nullable=False), | ||
| sa.Column('on_fail_action', sa.String(), nullable=False), | ||
| sa.Column( | ||
| "config", | ||
| postgresql.JSONB(astext_type=sa.Text()), | ||
| nullable=False, | ||
| server_default=sa.text("'{}'::jsonb"), | ||
| ), | ||
| sa.Column('is_enabled', sa.Boolean(), nullable=False, server_default=sa.true()), | ||
| sa.Column('created_at', sa.DateTime(), nullable=False), | ||
| sa.Column('updated_at', sa.DateTime(), nullable=False), | ||
|
|
||
| sa.PrimaryKeyConstraint('id'), | ||
| sa.UniqueConstraint('organization_id', 'project_id', 'type', 'stage', name='uq_validator_identity') | ||
| ) | ||
|
|
||
| op.create_index("idx_validator_organization", "validator_config", ["organization_id"]) | ||
| op.create_index("idx_validator_project", "validator_config", ["project_id"]) | ||
| op.create_index("idx_validator_type", "validator_config", ["type"]) | ||
| op.create_index("idx_validator_stage", "validator_config", ["stage"]) | ||
|
|
||
|
|
||
| def downgrade() -> None: | ||
| op.drop_table('validator_config') | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,11 @@ | ||
| from fastapi import APIRouter | ||
|
|
||
| from app.api.routes import utils, guardrails | ||
| from app.api.routes import utils, guardrails, validator_configs | ||
|
|
||
| api_router = APIRouter() | ||
| api_router.include_router(utils.router) | ||
| api_router.include_router(guardrails.router) | ||
| api_router.include_router(validator_configs.router) | ||
|
|
||
| # if settings.ENVIRONMENT == "local": | ||
| # api_router.include_router(private.router) |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,90 @@ | ||||||||||||||||||||||||||||||||||
| from typing import Optional | ||||||||||||||||||||||||||||||||||
| from uuid import UUID | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| from fastapi import APIRouter | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| from app.api.deps import AuthDep, SessionDep | ||||||||||||||||||||||||||||||||||
| from app.core.enum import Stage, ValidatorType | ||||||||||||||||||||||||||||||||||
| from app.schemas.validator_config import ValidatorCreate, ValidatorResponse, ValidatorUpdate | ||||||||||||||||||||||||||||||||||
| from app.crud.validator_config_crud import validator_config_crud | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| router = APIRouter( | ||||||||||||||||||||||||||||||||||
| prefix="/guardrails/validators/configs", | ||||||||||||||||||||||||||||||||||
| tags=["validator configs"], | ||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| @router.post( | ||||||||||||||||||||||||||||||||||
| "/", | ||||||||||||||||||||||||||||||||||
| response_model=ValidatorResponse | ||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be APIRESPONSE[ValidatorResponse] to maintain the consistency of response bodies being returned everywhere, and if you dont think that we need this here then why |
||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||
| async def create_validator( | ||||||||||||||||||||||||||||||||||
| payload: ValidatorCreate, | ||||||||||||||||||||||||||||||||||
| session: SessionDep, | ||||||||||||||||||||||||||||||||||
| organization_id: int, | ||||||||||||||||||||||||||||||||||
| project_id: int, | ||||||||||||||||||||||||||||||||||
| _: AuthDep, | ||||||||||||||||||||||||||||||||||
| ): | ||||||||||||||||||||||||||||||||||
| return validator_config_crud.create(session, organization_id, project_id, payload) | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+22
to
+29
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🌐 Web query:
💡 Result: Yes—calling a synchronous (blocking) database client inside an What to do instead (common patterns):
Important caveat: the thread pool is limited (Starlette’s default limiter is 40 “tokens”), so heavy sync DB usage can still bottleneck—just in the pool instead of the event loop. (starlette.dev) Citations:
🏁 Script executed: # Find SessionDep definition
rg -A 5 "SessionDep" backend/app --max-count 5Repository: ProjectTech4DevAI/kaapi-guardrails Length of output: 4390 🏁 Script executed: # Examine the validator_configs.py file to understand the imports and dependencies
cat -n backend/app/api/routes/validator_configs.py | head -40Repository: ProjectTech4DevAI/kaapi-guardrails Length of output: 1373 🏁 Script executed: # Find and inspect the validator_config_crud module
find backend -name "*crud*" -type f | head -20Repository: ProjectTech4DevAI/kaapi-guardrails Length of output: 119 🏁 Script executed: cat -n backend/app/api/deps.pyRepository: ProjectTech4DevAI/kaapi-guardrails Length of output: 1438 🏁 Script executed: cat -n backend/app/crud/validator_config_crud.pyRepository: ProjectTech4DevAI/kaapi-guardrails Length of output: 3920 Remove All five route handlers use Change all handlers to Suggested fix-async def create_validator(
+def create_validator(
payload: ValidatorCreate,
session: SessionDep,
organization_id: int,
project_id: int,
_: AuthDep,
):
return validator_config_crud.create(session, organization_id, project_id, payload)Apply the same change to 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
| @router.get( | ||||||||||||||||||||||||||||||||||
| "/", | ||||||||||||||||||||||||||||||||||
| response_model=list[ValidatorResponse] | ||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||
| async def list_validators( | ||||||||||||||||||||||||||||||||||
| organization_id: int, | ||||||||||||||||||||||||||||||||||
| project_id: int, | ||||||||||||||||||||||||||||||||||
| session: SessionDep, | ||||||||||||||||||||||||||||||||||
| _: AuthDep, | ||||||||||||||||||||||||||||||||||
| stage: Optional[Stage] = None, | ||||||||||||||||||||||||||||||||||
| type: Optional[ValidatorType] = None, | ||||||||||||||||||||||||||||||||||
| ): | ||||||||||||||||||||||||||||||||||
| return validator_config_crud.list(session, organization_id, project_id, stage, type) | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| @router.get( | ||||||||||||||||||||||||||||||||||
| "/{id}", | ||||||||||||||||||||||||||||||||||
| response_model=ValidatorResponse | ||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||
| async def get_validator( | ||||||||||||||||||||||||||||||||||
| id: UUID, | ||||||||||||||||||||||||||||||||||
| organization_id: int, | ||||||||||||||||||||||||||||||||||
| project_id: int, | ||||||||||||||||||||||||||||||||||
| session: SessionDep, | ||||||||||||||||||||||||||||||||||
| _: AuthDep, | ||||||||||||||||||||||||||||||||||
| ): | ||||||||||||||||||||||||||||||||||
| obj = validator_config_crud.get_or_404(session, id, organization_id, project_id) | ||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you can just let it ne "get" , it does not have to be "get or 404", thats making the name of crud function long for no reason |
||||||||||||||||||||||||||||||||||
| return validator_config_crud.flatten(obj) | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| @router.patch( | ||||||||||||||||||||||||||||||||||
| "/{id}", | ||||||||||||||||||||||||||||||||||
| response_model=ValidatorResponse | ||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||
| async def update_validator( | ||||||||||||||||||||||||||||||||||
| id: UUID, | ||||||||||||||||||||||||||||||||||
| organization_id: int, | ||||||||||||||||||||||||||||||||||
| project_id: int, | ||||||||||||||||||||||||||||||||||
| payload: ValidatorUpdate, | ||||||||||||||||||||||||||||||||||
| session: SessionDep, | ||||||||||||||||||||||||||||||||||
| _: AuthDep, | ||||||||||||||||||||||||||||||||||
| ): | ||||||||||||||||||||||||||||||||||
| obj = validator_config_crud.get_or_404(session, id, organization_id, project_id) | ||||||||||||||||||||||||||||||||||
| return validator_config_crud.update( | ||||||||||||||||||||||||||||||||||
| session, | ||||||||||||||||||||||||||||||||||
| obj, | ||||||||||||||||||||||||||||||||||
| payload.model_dump(exclude_unset=True), | ||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| @router.delete("/{id}") | ||||||||||||||||||||||||||||||||||
| async def delete_validator( | ||||||||||||||||||||||||||||||||||
| id: UUID, | ||||||||||||||||||||||||||||||||||
| organization_id: int, | ||||||||||||||||||||||||||||||||||
| project_id: int, | ||||||||||||||||||||||||||||||||||
| session: SessionDep, | ||||||||||||||||||||||||||||||||||
| _: AuthDep, | ||||||||||||||||||||||||||||||||||
| ): | ||||||||||||||||||||||||||||||||||
| obj = validator_config_crud.get_or_404(session, id, organization_id, project_id) | ||||||||||||||||||||||||||||||||||
| validator_config_crud.delete(session, obj) | ||||||||||||||||||||||||||||||||||
| return {"success": True} | ||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| from app.crud.request_log import RequestLogCrud | ||
| from app.crud.request_log_repo import RequestLogCrud | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why does it need to have the word "repo" here, it does not make sense to name crud files with the word repo |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Complete the alphabetical key ordering to resolve linter warnings.
The past review partially addressed the key ordering, but dotenv-linter still expects full alphabetical order. Currently POSTGRES_DB (line 13) should appear before POSTGRES_SERVER (line 12).
🔧 Complete alphabetical reordering fix
📝 Committable suggestion
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 13-13: [UnorderedKey] The POSTGRES_DB key should go before the POSTGRES_SERVER key
(UnorderedKey)
[warning] 14-14: [UnorderedKey] The POSTGRES_PORT key should go before the POSTGRES_SERVER key
(UnorderedKey)
[warning] 16-16: [UnorderedKey] The POSTGRES_PASSWORD key should go before the POSTGRES_PORT key
(UnorderedKey)
🤖 Prompt for AI Agents