fix(resource): ensure resource add remains atomic when discovery fails
This commit is contained in:
Forgejo
@@ -86,6 +86,13 @@ Feature: Resource CLI commands
|
||||
Then the resource command should fail
|
||||
And the resource output should contain "not found"
|
||||
|
||||
Scenario: Resource add rolls back when auto discovery fails
|
||||
Given built-in types are bootstrapped
|
||||
And auto discovery will fail during resource registration
|
||||
When I run resource add "git-checkout" "local/atomic-failure" with path "/tmp/atomic"
|
||||
Then the resource command should fail
|
||||
And the resource registry should not contain resource "local/atomic-failure"
|
||||
|
||||
# ---- Resource List ----
|
||||
|
||||
Scenario: List resources when empty
|
||||
|
||||
@@ -18,7 +18,11 @@ from sqlalchemy.orm import sessionmaker
|
||||
from cleveragents.application.services.resource_registry_service import (
|
||||
ResourceRegistryService,
|
||||
)
|
||||
from cleveragents.core.exceptions import CleverAgentsError, ValidationError
|
||||
from cleveragents.core.exceptions import (
|
||||
CleverAgentsError,
|
||||
NotFoundError,
|
||||
ValidationError,
|
||||
)
|
||||
from cleveragents.infrastructure.database.models import Base
|
||||
|
||||
|
||||
@@ -341,6 +345,18 @@ def step_set_resource_cli_format(context: Context, fmt: str) -> None:
|
||||
context.resource_cli_format = fmt
|
||||
|
||||
|
||||
@given("auto discovery will fail during resource registration")
|
||||
def step_auto_discovery_failure(context: Context) -> None:
|
||||
"""Force ResourceRepository.auto_discover_children to raise during the next call."""
|
||||
|
||||
patcher = patch(
|
||||
"cleveragents.infrastructure.database.repositories.ResourceRepository.auto_discover_children",
|
||||
side_effect=RuntimeError("Simulated auto-discovery failure"),
|
||||
)
|
||||
patcher.start()
|
||||
context.add_cleanup(patcher.stop)
|
||||
|
||||
|
||||
# ---- Resource List ----
|
||||
|
||||
|
||||
@@ -505,6 +521,18 @@ def step_resource_json_has_key(context: Context, key: str) -> None:
|
||||
raise AssertionError(f"Key '{key}' not found in JSON output")
|
||||
|
||||
|
||||
@then('the resource registry should not contain resource "{name}"')
|
||||
def step_registry_missing_resource(context: Context, name: str) -> None:
|
||||
"""Ensure that a resource was not persisted in the registry."""
|
||||
|
||||
service = _make_service(context)
|
||||
try:
|
||||
service.show_resource(name)
|
||||
except NotFoundError:
|
||||
return
|
||||
raise AssertionError(f"Resource '{name}' was persisted despite failure")
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Additional steps for resource_cli_coverage.feature
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -101,88 +101,85 @@ class ResourceInstanceMixin:
|
||||
"""
|
||||
session = self._session()
|
||||
try:
|
||||
type_row = (
|
||||
session.query(ResourceTypeModel).filter_by(name=type_name).first()
|
||||
)
|
||||
if type_row is None:
|
||||
raise NotFoundError(
|
||||
resource_type="resource_type",
|
||||
resource_id=type_name,
|
||||
with session.begin():
|
||||
type_row = (
|
||||
session.query(ResourceTypeModel).filter_by(name=type_name).first()
|
||||
)
|
||||
if type_row is None:
|
||||
raise NotFoundError(
|
||||
resource_type="resource_type",
|
||||
resource_id=type_name,
|
||||
)
|
||||
|
||||
if not type_row.user_addable:
|
||||
raise ValidationError(
|
||||
f"Resource type '{type_name}' is not user-addable. "
|
||||
"Only types with user_addable=true can be instantiated.",
|
||||
)
|
||||
|
||||
resource_kind_str: str = str(type_row.resource_kind)
|
||||
resource_id = str(ULID())
|
||||
|
||||
# Determine namespace from the resource name
|
||||
namespace: str | None = None
|
||||
if name is not None:
|
||||
parts = name.split("/", 1)
|
||||
if len(parts) == 2:
|
||||
namespace = parts[0]
|
||||
|
||||
now_iso = datetime.now(tz=UTC).isoformat()
|
||||
properties_json: str | None = None
|
||||
if properties:
|
||||
properties_json = json.dumps(properties)
|
||||
|
||||
db_resource = ResourceModel(
|
||||
resource_id=resource_id,
|
||||
namespaced_name=name,
|
||||
namespace=namespace,
|
||||
type_name=type_name,
|
||||
resource_kind=resource_kind_str,
|
||||
location=location,
|
||||
description=description,
|
||||
read_only=read_only,
|
||||
auto_discovered=False,
|
||||
sandbox_strategy=None,
|
||||
content_hash=None,
|
||||
properties_json=properties_json,
|
||||
metadata_json=None,
|
||||
created_at=now_iso,
|
||||
updated_at=now_iso,
|
||||
)
|
||||
|
||||
if not type_row.user_addable:
|
||||
raise ValidationError(
|
||||
f"Resource type '{type_name}' is not user-addable. "
|
||||
"Only types with user_addable=true can be instantiated.",
|
||||
session.add(db_resource)
|
||||
session.flush()
|
||||
|
||||
repository = ResourceRepository(self._session)
|
||||
try:
|
||||
repository.auto_discover_children(
|
||||
resource_id,
|
||||
session=session,
|
||||
commit=False,
|
||||
)
|
||||
except Exception as auto_exc:
|
||||
logger.error(
|
||||
"Auto-discovery failed; rolling back parent resource",
|
||||
extra={"resource_id": resource_id, "type_name": type_name},
|
||||
)
|
||||
raise
|
||||
|
||||
return Resource(
|
||||
resource_id=resource_id,
|
||||
name=name,
|
||||
resource_type_name=type_name,
|
||||
classification=PhysVirt(resource_kind_str),
|
||||
description=description,
|
||||
properties=properties or {},
|
||||
location=location,
|
||||
content_hash=None,
|
||||
sandbox_strategy=None,
|
||||
created_at=datetime.fromisoformat(now_iso),
|
||||
updated_at=datetime.fromisoformat(now_iso),
|
||||
)
|
||||
|
||||
resource_kind_str: str = str(type_row.resource_kind)
|
||||
resource_id = str(ULID())
|
||||
|
||||
# Determine namespace from the resource name
|
||||
namespace: str | None = None
|
||||
if name is not None:
|
||||
parts = name.split("/", 1)
|
||||
if len(parts) == 2:
|
||||
namespace = parts[0]
|
||||
|
||||
now_iso = datetime.now(tz=UTC).isoformat()
|
||||
properties_json: str | None = None
|
||||
if properties:
|
||||
properties_json = json.dumps(properties)
|
||||
|
||||
db_resource = ResourceModel(
|
||||
resource_id=resource_id,
|
||||
namespaced_name=name,
|
||||
namespace=namespace,
|
||||
type_name=type_name,
|
||||
resource_kind=resource_kind_str,
|
||||
location=location,
|
||||
description=description,
|
||||
read_only=read_only,
|
||||
auto_discovered=False,
|
||||
sandbox_strategy=None,
|
||||
content_hash=None,
|
||||
properties_json=properties_json,
|
||||
metadata_json=None,
|
||||
created_at=now_iso,
|
||||
updated_at=now_iso,
|
||||
)
|
||||
|
||||
session.add(db_resource)
|
||||
session.flush()
|
||||
|
||||
repository = ResourceRepository(self._session)
|
||||
try:
|
||||
repository.auto_discover_children(
|
||||
resource_id,
|
||||
session=session,
|
||||
commit=False,
|
||||
)
|
||||
except Exception as auto_exc:
|
||||
logger.error(
|
||||
"Auto-discovery failed; rolling back parent resource",
|
||||
extra={"resource_id": resource_id, "type_name": type_name},
|
||||
)
|
||||
session.rollback()
|
||||
raise
|
||||
|
||||
session.commit()
|
||||
|
||||
resource = Resource(
|
||||
resource_id=resource_id,
|
||||
name=name,
|
||||
resource_type_name=type_name,
|
||||
classification=PhysVirt(resource_kind_str),
|
||||
description=description,
|
||||
properties=properties or {},
|
||||
location=location,
|
||||
content_hash=None,
|
||||
sandbox_strategy=None,
|
||||
created_at=datetime.fromisoformat(now_iso),
|
||||
updated_at=datetime.fromisoformat(now_iso),
|
||||
)
|
||||
return resource
|
||||
except (NotFoundError, ValidationError):
|
||||
session.rollback()
|
||||
raise
|
||||
|
||||
Reference in New Issue
Block a user