chore(BA-1005): Migrate to pydantic in manager configuration #3994
+1,265
−0
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Pull Request Overview
This PR migrates the manager configuration to use pydantic by introducing new models for managing volume configuration.
- Added pydantic models for virtual folder types (VFolderTypeConfig) and proxies (VFolderProxyConfig).
- Introduced a VolumeConfig model consolidating configuration details for virtual folders and volume proxies.
Comments suppressed due to low confidence (1)
src/ai/backend/manager/config/volume.py:72
- [nitpick] The default factory for '_types' in VolumeConfig only sets a default value for 'user' but leaves 'group' as None. If both configurations are intended to be initialized similarly, consider providing a default for 'group' as well.
default_factory=lambda: VFolderTypeConfig(user={}),
HyeockJinKim
changed the title
HyeockJinKim
force-pushed
the
refactor/move-to-pydantic-config-manager
branch
from
667c5cc to
01796c3
Compare
fregataa
reviewed
Mar 24, 2025
Comment on lines
+83
to
+111
| pool_size: int = Field( | ||
| default=8, | ||
| ge=1, | ||
| description=""" | ||
| Size of the database connection pool. | ||
| Determines how many concurrent database connections to maintain. | ||
| Should be tuned based on expected load and database server capacity. | ||
| """, | ||
| examples=[1, 8], | ||
| ) | ||
| pool_recycle: float = Field( | ||
| default=-1, | ||
| ge=-1, | ||
| description=""" | ||
| Maximum lifetime of a connection in seconds before it's recycled. | ||
| Set to -1 to disable connection recycling. | ||
| Useful to handle cases where database connections are closed by the server after inactivity. | ||
| """, | ||
| examples=[-1, 50], | ||
| ) | ||
| pool_pre_ping: bool = Field( | ||
| default=False, | ||
| description=""" | ||
| Whether to test connections with a lightweight ping before using them. | ||
| Helps detect stale connections before they cause application errors. | ||
| Adds a small overhead but improves reliability. | ||
| """, | ||
| examples=[True], | ||
| ) |
There was a problem hiding this comment.
Could you leave the SQLAlchemy reference of the settings?
More refs are here.
pool_recycle , pool_pre_ping
| default=False, | ||
| description=""" | ||
| Whether to use the experimental Redis-based event dispatcher. | ||
| May provide better performance for event handling in large clusters. |
There was a problem hiding this comment.
The reason we implemented this dispatcher is because the redis-py async module seems to have issues. (It is not because of performance)
There was a problem hiding this comment.
Most of the descriptions are written by the copilot, but I'll check the whole thing once.
| Limits the complexity of queries to prevent abuse. | ||
| Set to None to disable the limit. | ||
| """, | ||
| examples=[None, 10, 15], |
There was a problem hiding this comment.
How does pydantic.Field convert null value to None? Users should be able to set null values through config explicitly
HyeockJinKim
force-pushed
the
refactor/move-to-pydantic-config-manager
branch
from
459a517 to
7e93d9d
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
resolves #3993 (BA-1005)
Checklist: (if applicable)
ai.backend.testdocsdirectory