-
Notifications
You must be signed in to change notification settings - Fork 0
feat(settings): register API config in SettingsService with 2-phase init #518
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
Changes from all commits
2f25ee7
1e69dc2
d7fd554
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,159 @@ | ||
| """API namespace setting definitions. | ||
|
|
||
| Registers 10 settings covering server, CORS, rate limiting, and | ||
| authentication. Four are runtime-editable; six are bootstrap-only | ||
| (``restart_required=True``) because Litestar bakes middleware and | ||
| CORS into the application at construction time. | ||
| """ | ||
|
|
||
| from synthorg.settings.enums import SettingLevel, SettingNamespace, SettingType | ||
| from synthorg.settings.models import SettingDefinition | ||
| from synthorg.settings.registry import get_registry | ||
|
|
||
| _r = get_registry() | ||
|
|
||
| # ── Server (bootstrap-only) ────────────────────────────────────── | ||
|
|
||
| _r.register( | ||
| SettingDefinition( | ||
| namespace=SettingNamespace.API, | ||
| key="server_host", | ||
| type=SettingType.STRING, | ||
| default="127.0.0.1", | ||
| description="Server bind address", | ||
| group="Server", | ||
| restart_required=True, | ||
| yaml_path="api.server.host", | ||
| ) | ||
| ) | ||
|
|
||
| _r.register( | ||
| SettingDefinition( | ||
| namespace=SettingNamespace.API, | ||
| key="server_port", | ||
| type=SettingType.INTEGER, | ||
| default="8000", | ||
| description="Server bind port", | ||
| group="Server", | ||
| restart_required=True, | ||
| min_value=1, | ||
| max_value=65535, | ||
| yaml_path="api.server.port", | ||
| ) | ||
| ) | ||
|
|
||
| _r.register( | ||
| SettingDefinition( | ||
| namespace=SettingNamespace.API, | ||
| key="api_prefix", | ||
| type=SettingType.STRING, | ||
| default="/api/v1", | ||
| description="URL prefix for all API routes", | ||
| group="Server", | ||
| level=SettingLevel.ADVANCED, | ||
| restart_required=True, | ||
| yaml_path="api.api_prefix", | ||
| ) | ||
|
Comment on lines
+45
to
+56
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. These bootstrap keys are registered in the settings DB chain, but the current startup path never reads them from settings.
Also applies to: 61-71, 103-114, 147-158 🤖 Prompt for AI Agents |
||
| ) | ||
|
|
||
| # ── CORS (bootstrap-only) ──────────────────────────────────────── | ||
|
|
||
| _r.register( | ||
| SettingDefinition( | ||
| namespace=SettingNamespace.API, | ||
| key="cors_allowed_origins", | ||
| type=SettingType.JSON, | ||
| default='["http://localhost:5173"]', | ||
| description="Origins permitted to make cross-origin requests", | ||
| group="CORS", | ||
| restart_required=True, | ||
| yaml_path="api.cors.allowed_origins", | ||
| ) | ||
|
Comment on lines
+62
to
+71
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 setting is missing the SettingDefinition(
namespace=SettingNamespace.API,
key="cors_allowed_origins",
type=SettingType.JSON,
default='["http://localhost:5173"]',
description="Origins permitted to make cross-origin requests",
group="CORS",
restart_required=True,
yaml_path="api.cors.allowed_origins",
) |
||
| ) | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| # ── Rate Limiting (exclude_paths: bootstrap-only) ──────────────── | ||
|
|
||
| _r.register( | ||
| SettingDefinition( | ||
| namespace=SettingNamespace.API, | ||
| key="rate_limit_max_requests", | ||
| type=SettingType.INTEGER, | ||
| default="100", | ||
| description="Maximum requests per time window", | ||
| group="Rate Limiting", | ||
| min_value=1, | ||
| max_value=10000, | ||
| yaml_path="api.rate_limit.max_requests", | ||
| ) | ||
| ) | ||
|
|
||
| _r.register( | ||
| SettingDefinition( | ||
| namespace=SettingNamespace.API, | ||
| key="rate_limit_time_unit", | ||
| type=SettingType.ENUM, | ||
| default="minute", | ||
| description="Rate limit time window", | ||
| group="Rate Limiting", | ||
| enum_values=("second", "minute", "hour", "day"), | ||
| yaml_path="api.rate_limit.time_unit", | ||
| ) | ||
|
Comment on lines
+76
to
+100
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. These two settings are still restart-only in practice. The new note in 🔧 Minimal metadata fix _r.register(
SettingDefinition(
namespace=SettingNamespace.API,
key="rate_limit_max_requests",
type=SettingType.INTEGER,
default="100",
description="Maximum requests per time window",
group="Rate Limiting",
+ restart_required=True,
min_value=1,
max_value=10000,
yaml_path="api.rate_limit.max_requests",
)
)
@@
_r.register(
SettingDefinition(
namespace=SettingNamespace.API,
key="rate_limit_time_unit",
type=SettingType.ENUM,
default="minute",
description="Rate limit time window",
group="Rate Limiting",
+ restart_required=True,
enum_values=("second", "minute", "hour", "day"),
yaml_path="api.rate_limit.time_unit",
)
)🤖 Prompt for AI Agents |
||
| ) | ||
|
|
||
| _r.register( | ||
| SettingDefinition( | ||
| namespace=SettingNamespace.API, | ||
| key="rate_limit_exclude_paths", | ||
| type=SettingType.JSON, | ||
| default='["/api/v1/health"]', | ||
| description="Paths excluded from rate limiting", | ||
| group="Rate Limiting", | ||
| level=SettingLevel.ADVANCED, | ||
| restart_required=True, | ||
| yaml_path="api.rate_limit.exclude_paths", | ||
| ) | ||
|
Comment on lines
+104
to
+114
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 setting is missing the SettingDefinition(
namespace=SettingNamespace.API,
key="rate_limit_exclude_paths",
type=SettingType.JSON,
default='["/api/v1/health"]',
description="Paths excluded from rate limiting",
group="Rate Limiting",
level=SettingLevel.ADVANCED,
restart_required=True,
yaml_path="api.rate_limit.exclude_paths",
) |
||
| ) | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| # ── Authentication (exclude_paths: bootstrap-only) ─────────────── | ||
|
|
||
| _r.register( | ||
| SettingDefinition( | ||
| namespace=SettingNamespace.API, | ||
| key="jwt_expiry_minutes", | ||
| type=SettingType.INTEGER, | ||
| default="1440", | ||
| description="JWT token lifetime in minutes", | ||
| group="Authentication", | ||
| min_value=1, | ||
| max_value=10080, | ||
| yaml_path="api.auth.jwt_expiry_minutes", | ||
| ) | ||
| ) | ||
|
|
||
| _r.register( | ||
| SettingDefinition( | ||
| namespace=SettingNamespace.API, | ||
| key="min_password_length", | ||
| type=SettingType.INTEGER, | ||
| default="12", | ||
| description="Minimum password length for setup and password change", | ||
| group="Authentication", | ||
| min_value=12, | ||
| max_value=128, | ||
| yaml_path="api.auth.min_password_length", | ||
| ) | ||
| ) | ||
|
|
||
| _r.register( | ||
| SettingDefinition( | ||
| namespace=SettingNamespace.API, | ||
| key="auth_exclude_paths", | ||
| type=SettingType.JSON, | ||
| default="[]", | ||
| description="Paths excluded from authentication middleware", | ||
| group="Authentication", | ||
| level=SettingLevel.ADVANCED, | ||
| restart_required=True, | ||
| yaml_path="api.auth.exclude_paths", | ||
| ) | ||
|
Comment on lines
+148
to
+158
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 setting is missing the SettingDefinition(
namespace=SettingNamespace.API,
key="auth_exclude_paths",
type=SettingType.JSON,
default="[]",
description="Paths excluded from authentication middleware",
group="Authentication",
level=SettingLevel.ADVANCED,
restart_required=True,
yaml_path="api.auth.exclude_paths",
) |
||
| ) | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
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.
The auth settings called “runtime-editable” never reach the active
AuthService.This note says
jwt_expiry_minutesandmin_password_lengthresolve post-startup, but_init_persistence()still constructsAuthServicedirectly fromapp_state.config.api.auth. With the current flow, pre-existing DB overrides for those fields are ignored unless the auth service is built fromConfigResolver.get_api_config()(or otherwise refreshed from settings after startup).🤖 Prompt for AI Agents