mirror of
https://github.com/GNS3/gns3-server.git
synced 2026-10-25 11:22:26 +02:00
feat(api): add optimistic locking to LLM model config updates
- Include `version` field in all LLM model config response schemas - Add `expected_version` parameter to update endpoints for optimistic locking - Handle concurrent modification errors with HTTP 409 Conflict status - Update both user and group config endpoints consistently
This commit is contained in:
parent
9315f06e1f
commit
161b7feb01
@ -101,6 +101,7 @@ async def get_user_own_llm_model_configs(
|
|||||||
user_id=config.user_id,
|
user_id=config.user_id,
|
||||||
group_id=config.group_id,
|
group_id=config.group_id,
|
||||||
is_default=config.is_default,
|
is_default=config.is_default,
|
||||||
|
version=config.version,
|
||||||
created_at=config.created_at,
|
created_at=config.created_at,
|
||||||
updated_at=config.updated_at
|
updated_at=config.updated_at
|
||||||
)
|
)
|
||||||
@ -151,6 +152,7 @@ async def create_user_llm_model_config(
|
|||||||
user_id=new_config.user_id,
|
user_id=new_config.user_id,
|
||||||
group_id=new_config.group_id,
|
group_id=new_config.group_id,
|
||||||
is_default=new_config.is_default,
|
is_default=new_config.is_default,
|
||||||
|
version=new_config.version,
|
||||||
created_at=new_config.created_at,
|
created_at=new_config.created_at,
|
||||||
updated_at=new_config.updated_at
|
updated_at=new_config.updated_at
|
||||||
)
|
)
|
||||||
@ -177,6 +179,7 @@ async def update_user_llm_model_config(
|
|||||||
) -> schemas.LLMModelConfigResponse:
|
) -> schemas.LLMModelConfigResponse:
|
||||||
"""
|
"""
|
||||||
Update a user's LLM model configuration.
|
Update a user's LLM model configuration.
|
||||||
|
Supports optimistic locking via expected_version field.
|
||||||
|
|
||||||
Required privilege: User.Modify
|
Required privilege: User.Modify
|
||||||
"""
|
"""
|
||||||
@ -185,7 +188,15 @@ async def update_user_llm_model_config(
|
|||||||
# Build updates dict with only non-None values
|
# Build updates dict with only non-None values
|
||||||
updates = {k: v for k, v in config_update.model_dump().items() if v is not None}
|
updates = {k: v for k, v in config_update.model_dump().items() if v is not None}
|
||||||
|
|
||||||
updated_config = await llm_repo.update_user_config(config_id, user_id, updates)
|
# Extract expected_version for optimistic locking
|
||||||
|
expected_version = updates.pop("expected_version", None)
|
||||||
|
|
||||||
|
updated_config = await llm_repo.update_user_config(
|
||||||
|
config_id,
|
||||||
|
user_id,
|
||||||
|
updates,
|
||||||
|
expected_version=expected_version
|
||||||
|
)
|
||||||
|
|
||||||
if not updated_config:
|
if not updated_config:
|
||||||
raise HTTPException(
|
raise HTTPException(
|
||||||
@ -199,11 +210,20 @@ async def update_user_llm_model_config(
|
|||||||
user_id=updated_config.user_id,
|
user_id=updated_config.user_id,
|
||||||
group_id=updated_config.group_id,
|
group_id=updated_config.group_id,
|
||||||
is_default=updated_config.is_default,
|
is_default=updated_config.is_default,
|
||||||
|
version=updated_config.version,
|
||||||
created_at=updated_config.created_at,
|
created_at=updated_config.created_at,
|
||||||
updated_at=updated_config.updated_at
|
updated_at=updated_config.updated_at
|
||||||
)
|
)
|
||||||
except HTTPException:
|
except HTTPException:
|
||||||
raise
|
raise
|
||||||
|
except ValueError as e:
|
||||||
|
# Handle optimistic lock errors
|
||||||
|
if "Concurrent modification" in str(e):
|
||||||
|
raise HTTPException(
|
||||||
|
status_code=status.HTTP_409_CONFLICT,
|
||||||
|
detail=str(e)
|
||||||
|
)
|
||||||
|
raise ControllerBadRequestError(str(e))
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
log.error(f"Failed to update LLM model config: {e}")
|
log.error(f"Failed to update LLM model config: {e}")
|
||||||
raise HTTPException(
|
raise HTTPException(
|
||||||
@ -277,6 +297,7 @@ async def set_user_default_llm_model_config(
|
|||||||
user_id=config.user_id,
|
user_id=config.user_id,
|
||||||
group_id=config.group_id,
|
group_id=config.group_id,
|
||||||
is_default=config.is_default,
|
is_default=config.is_default,
|
||||||
|
version=config.version,
|
||||||
created_at=config.created_at,
|
created_at=config.created_at,
|
||||||
updated_at=config.updated_at
|
updated_at=config.updated_at
|
||||||
)
|
)
|
||||||
@ -318,6 +339,7 @@ async def get_group_llm_model_configs(
|
|||||||
user_id=config.user_id,
|
user_id=config.user_id,
|
||||||
group_id=config.group_id,
|
group_id=config.group_id,
|
||||||
is_default=config.is_default,
|
is_default=config.is_default,
|
||||||
|
version=config.version,
|
||||||
created_at=config.created_at,
|
created_at=config.created_at,
|
||||||
updated_at=config.updated_at
|
updated_at=config.updated_at
|
||||||
)
|
)
|
||||||
@ -368,6 +390,7 @@ async def create_group_llm_model_config(
|
|||||||
user_id=new_config.user_id,
|
user_id=new_config.user_id,
|
||||||
group_id=new_config.group_id,
|
group_id=new_config.group_id,
|
||||||
is_default=new_config.is_default,
|
is_default=new_config.is_default,
|
||||||
|
version=new_config.version,
|
||||||
created_at=new_config.created_at,
|
created_at=new_config.created_at,
|
||||||
updated_at=new_config.updated_at
|
updated_at=new_config.updated_at
|
||||||
)
|
)
|
||||||
@ -394,6 +417,7 @@ async def update_group_llm_model_config(
|
|||||||
) -> schemas.LLMModelConfigResponse:
|
) -> schemas.LLMModelConfigResponse:
|
||||||
"""
|
"""
|
||||||
Update a group's LLM model configuration.
|
Update a group's LLM model configuration.
|
||||||
|
Supports optimistic locking via expected_version field.
|
||||||
|
|
||||||
Required privilege: Group.Modify
|
Required privilege: Group.Modify
|
||||||
"""
|
"""
|
||||||
@ -402,7 +426,15 @@ async def update_group_llm_model_config(
|
|||||||
# Build updates dict with only non-None values
|
# Build updates dict with only non-None values
|
||||||
updates = {k: v for k, v in config_update.model_dump().items() if v is not None}
|
updates = {k: v for k, v in config_update.model_dump().items() if v is not None}
|
||||||
|
|
||||||
updated_config = await llm_repo.update_group_config(config_id, group_id, updates)
|
# Extract expected_version for optimistic locking
|
||||||
|
expected_version = updates.pop("expected_version", None)
|
||||||
|
|
||||||
|
updated_config = await llm_repo.update_group_config(
|
||||||
|
config_id,
|
||||||
|
group_id,
|
||||||
|
updates,
|
||||||
|
expected_version=expected_version
|
||||||
|
)
|
||||||
|
|
||||||
if not updated_config:
|
if not updated_config:
|
||||||
raise HTTPException(
|
raise HTTPException(
|
||||||
@ -416,11 +448,20 @@ async def update_group_llm_model_config(
|
|||||||
user_id=updated_config.user_id,
|
user_id=updated_config.user_id,
|
||||||
group_id=updated_config.group_id,
|
group_id=updated_config.group_id,
|
||||||
is_default=updated_config.is_default,
|
is_default=updated_config.is_default,
|
||||||
|
version=updated_config.version,
|
||||||
created_at=updated_config.created_at,
|
created_at=updated_config.created_at,
|
||||||
updated_at=updated_config.updated_at
|
updated_at=updated_config.updated_at
|
||||||
)
|
)
|
||||||
except HTTPException:
|
except HTTPException:
|
||||||
raise
|
raise
|
||||||
|
except ValueError as e:
|
||||||
|
# Handle optimistic lock errors
|
||||||
|
if "Concurrent modification" in str(e):
|
||||||
|
raise HTTPException(
|
||||||
|
status_code=status.HTTP_409_CONFLICT,
|
||||||
|
detail=str(e)
|
||||||
|
)
|
||||||
|
raise ControllerBadRequestError(str(e))
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
log.error(f"Failed to update LLM model config: {e}")
|
log.error(f"Failed to update LLM model config: {e}")
|
||||||
raise HTTPException(
|
raise HTTPException(
|
||||||
@ -494,6 +535,7 @@ async def set_group_default_llm_model_config(
|
|||||||
user_id=config.user_id,
|
user_id=config.user_id,
|
||||||
group_id=config.group_id,
|
group_id=config.group_id,
|
||||||
is_default=config.is_default,
|
is_default=config.is_default,
|
||||||
|
version=config.version,
|
||||||
created_at=config.created_at,
|
created_at=config.created_at,
|
||||||
updated_at=config.updated_at
|
updated_at=config.updated_at
|
||||||
)
|
)
|
||||||
|
|||||||
@ -15,7 +15,7 @@
|
|||||||
# You should have received a copy of the GNU General Public License
|
# You should have received a copy of the GNU General Public License
|
||||||
# along with this program. If not, see <http://www.gnu.org/licenses/>.
|
# along with this program. If not, see <http://www.gnu.org/licenses/>.
|
||||||
|
|
||||||
from sqlalchemy import Column, Boolean, ForeignKey, CheckConstraint, UniqueConstraint, Index
|
from sqlalchemy import Column, Boolean, ForeignKey, CheckConstraint, UniqueConstraint, Index, Integer
|
||||||
from sqlalchemy.dialects.postgresql import JSONB
|
from sqlalchemy.dialects.postgresql import JSONB
|
||||||
from sqlalchemy.orm import relationship
|
from sqlalchemy.orm import relationship
|
||||||
|
|
||||||
@ -39,6 +39,7 @@ class LLMModelConfig(BaseTable):
|
|||||||
user_id = Column(GUID, ForeignKey("users.user_id", ondelete="CASCADE"), nullable=True)
|
user_id = Column(GUID, ForeignKey("users.user_id", ondelete="CASCADE"), nullable=True)
|
||||||
group_id = Column(GUID, ForeignKey("user_groups.user_group_id", ondelete="CASCADE"), nullable=True)
|
group_id = Column(GUID, ForeignKey("user_groups.user_group_id", ondelete="CASCADE"), nullable=True)
|
||||||
is_default = Column(Boolean, default=False, nullable=False)
|
is_default = Column(Boolean, default=False, nullable=False)
|
||||||
|
version = Column(Integer, default=0, nullable=False) # Optimistic locking version
|
||||||
|
|
||||||
# Relationships
|
# Relationships
|
||||||
user = relationship("User", backref="llm_model_configs")
|
user = relationship("User", backref="llm_model_configs")
|
||||||
@ -69,4 +70,4 @@ class LLMModelConfig(BaseTable):
|
|||||||
def __repr__(self):
|
def __repr__(self):
|
||||||
config_name = self.config.get("name", "unnamed") if self.config else "unnamed"
|
config_name = self.config.get("name", "unnamed") if self.config else "unnamed"
|
||||||
owner = f"user_{self.user_id}" if self.user_id else f"group_{self.group_id}"
|
owner = f"user_{self.user_id}" if self.user_id else f"group_{self.group_id}"
|
||||||
return f"<LLMModelConfig {config_name} for {owner} (default={self.is_default})>"
|
return f"<LLMModelConfig {config_name} for {owner} (default={self.is_default}, version={self.version})>"
|
||||||
|
|||||||
@ -96,9 +96,19 @@ class LLMModelConfigsRepository(BaseRepository):
|
|||||||
self,
|
self,
|
||||||
config_id: UUID,
|
config_id: UUID,
|
||||||
user_id: UUID,
|
user_id: UUID,
|
||||||
updates: Dict[str, Any]
|
updates: Dict[str, Any],
|
||||||
|
expected_version: Optional[int] = None
|
||||||
) -> Optional[models.LLMModelConfig]:
|
) -> Optional[models.LLMModelConfig]:
|
||||||
"""Update a user's LLM model configuration."""
|
"""
|
||||||
|
Update a user's LLM model configuration.
|
||||||
|
Uses optimistic locking to prevent concurrent modifications.
|
||||||
|
|
||||||
|
:param config_id: Configuration ID
|
||||||
|
:param user_id: User ID
|
||||||
|
:param updates: Dictionary of fields to update
|
||||||
|
:param expected_version: Expected version for optimistic locking (raises error if mismatch)
|
||||||
|
:raises ValueError: If version mismatch (concurrent modification)
|
||||||
|
"""
|
||||||
query = select(models.LLMModelConfig).where(
|
query = select(models.LLMModelConfig).where(
|
||||||
and_(
|
and_(
|
||||||
models.LLMModelConfig.config_id == config_id,
|
models.LLMModelConfig.config_id == config_id,
|
||||||
@ -111,6 +121,13 @@ class LLMModelConfigsRepository(BaseRepository):
|
|||||||
if not db_config:
|
if not db_config:
|
||||||
return None
|
return None
|
||||||
|
|
||||||
|
# Check optimistic lock if expected_version is provided
|
||||||
|
if expected_version is not None and db_config.version != expected_version:
|
||||||
|
raise ValueError(
|
||||||
|
f"Concurrent modification detected. Expected version {expected_version}, "
|
||||||
|
f"but current version is {db_config.version}. Please retry."
|
||||||
|
)
|
||||||
|
|
||||||
# Encrypt API key if present in updates
|
# Encrypt API key if present in updates
|
||||||
from gns3server.utils.encryption import encrypt
|
from gns3server.utils.encryption import encrypt
|
||||||
updates_copy = updates.copy()
|
updates_copy = updates.copy()
|
||||||
@ -130,6 +147,8 @@ class LLMModelConfigsRepository(BaseRepository):
|
|||||||
current_config[key] = value
|
current_config[key] = value
|
||||||
|
|
||||||
db_config.config = current_config
|
db_config.config = current_config
|
||||||
|
# Increment version for optimistic locking
|
||||||
|
db_config.version = db_config.version + 1
|
||||||
await self._db_session.commit()
|
await self._db_session.commit()
|
||||||
await self._db_session.refresh(db_config)
|
await self._db_session.refresh(db_config)
|
||||||
return db_config
|
return db_config
|
||||||
@ -234,9 +253,19 @@ class LLMModelConfigsRepository(BaseRepository):
|
|||||||
self,
|
self,
|
||||||
config_id: UUID,
|
config_id: UUID,
|
||||||
group_id: UUID,
|
group_id: UUID,
|
||||||
updates: Dict[str, Any]
|
updates: Dict[str, Any],
|
||||||
|
expected_version: Optional[int] = None
|
||||||
) -> Optional[models.LLMModelConfig]:
|
) -> Optional[models.LLMModelConfig]:
|
||||||
"""Update a group's LLM model configuration."""
|
"""
|
||||||
|
Update a group's LLM model configuration.
|
||||||
|
Uses optimistic locking to prevent concurrent modifications.
|
||||||
|
|
||||||
|
:param config_id: Configuration ID
|
||||||
|
:param group_id: Group ID
|
||||||
|
:param updates: Dictionary of fields to update
|
||||||
|
:param expected_version: Expected version for optimistic locking (raises error if mismatch)
|
||||||
|
:raises ValueError: If version mismatch (concurrent modification)
|
||||||
|
"""
|
||||||
query = select(models.LLMModelConfig).where(
|
query = select(models.LLMModelConfig).where(
|
||||||
and_(
|
and_(
|
||||||
models.LLMModelConfig.config_id == config_id,
|
models.LLMModelConfig.config_id == config_id,
|
||||||
@ -249,6 +278,13 @@ class LLMModelConfigsRepository(BaseRepository):
|
|||||||
if not db_config:
|
if not db_config:
|
||||||
return None
|
return None
|
||||||
|
|
||||||
|
# Check optimistic lock if expected_version is provided
|
||||||
|
if expected_version is not None and db_config.version != expected_version:
|
||||||
|
raise ValueError(
|
||||||
|
f"Concurrent modification detected. Expected version {expected_version}, "
|
||||||
|
f"but current version is {db_config.version}. Please retry."
|
||||||
|
)
|
||||||
|
|
||||||
# Encrypt API key if present in updates
|
# Encrypt API key if present in updates
|
||||||
from gns3server.utils.encryption import encrypt
|
from gns3server.utils.encryption import encrypt
|
||||||
updates_copy = updates.copy()
|
updates_copy = updates.copy()
|
||||||
@ -268,6 +304,8 @@ class LLMModelConfigsRepository(BaseRepository):
|
|||||||
current_config[key] = value
|
current_config[key] = value
|
||||||
|
|
||||||
db_config.config = current_config
|
db_config.config = current_config
|
||||||
|
# Increment version for optimistic locking
|
||||||
|
db_config.version = db_config.version + 1
|
||||||
await self._db_session.commit()
|
await self._db_session.commit()
|
||||||
await self._db_session.refresh(db_config)
|
await self._db_session.refresh(db_config)
|
||||||
return db_config
|
return db_config
|
||||||
|
|||||||
@ -37,6 +37,7 @@ def upgrade() -> None:
|
|||||||
sa.Column('user_id', postgresql.UUID(as_uuid=True), sa.ForeignKey('users.user_id', ondelete='CASCADE'), nullable=True),
|
sa.Column('user_id', postgresql.UUID(as_uuid=True), sa.ForeignKey('users.user_id', ondelete='CASCADE'), nullable=True),
|
||||||
sa.Column('group_id', postgresql.UUID(as_uuid=True), sa.ForeignKey('user_groups.user_group_id', ondelete='CASCADE'), nullable=True),
|
sa.Column('group_id', postgresql.UUID(as_uuid=True), sa.ForeignKey('user_groups.user_group_id', ondelete='CASCADE'), nullable=True),
|
||||||
sa.Column('is_default', sa.Boolean(), default=False, nullable=False),
|
sa.Column('is_default', sa.Boolean(), default=False, nullable=False),
|
||||||
|
sa.Column('version', sa.Integer(), nullable=False, server_default='0'),
|
||||||
sa.Column('created_at', sa.DateTime(), nullable=True),
|
sa.Column('created_at', sa.DateTime(), nullable=True),
|
||||||
sa.Column('updated_at', sa.DateTime(), nullable=True),
|
sa.Column('updated_at', sa.DateTime(), nullable=True),
|
||||||
sa.CheckConstraint(
|
sa.CheckConstraint(
|
||||||
|
|||||||
@ -58,6 +58,7 @@ class LLMModelConfigUpdate(BaseModel):
|
|||||||
api_key: Optional[str] = None
|
api_key: Optional[str] = None
|
||||||
max_tokens: Optional[int] = Field(None, gt=0)
|
max_tokens: Optional[int] = Field(None, gt=0)
|
||||||
is_default: Optional[bool] = None
|
is_default: Optional[bool] = None
|
||||||
|
expected_version: Optional[int] = Field(None, description="Expected version for optimistic locking")
|
||||||
|
|
||||||
# Allow extra fields for extensibility
|
# Allow extra fields for extensibility
|
||||||
model_config = ConfigDict(extra="allow")
|
model_config = ConfigDict(extra="allow")
|
||||||
@ -72,6 +73,7 @@ class LLMModelConfigResponse(DateTimeModelMixin):
|
|||||||
user_id: Optional[UUID] = None
|
user_id: Optional[UUID] = None
|
||||||
group_id: Optional[UUID] = None
|
group_id: Optional[UUID] = None
|
||||||
is_default: bool
|
is_default: bool
|
||||||
|
version: int = Field(..., description="Optimistic locking version")
|
||||||
|
|
||||||
model_config = ConfigDict(from_attributes=True)
|
model_config = ConfigDict(from_attributes=True)
|
||||||
|
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user