Skip to content

Commit

Permalink
Merge pull request #1749 from dandi/exception-name
Browse files Browse the repository at this point in the history
  • Loading branch information
mvandenburgh authored Nov 27, 2023
2 parents 3fb65ed + 70e015b commit 63cc75e
Show file tree
Hide file tree
Showing 16 changed files with 85 additions and 82 deletions.
4 changes: 2 additions & 2 deletions dandiapi/api/asset_paths.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,12 +125,12 @@ def insert_asset_paths(asset: Asset, version: Version):
path=asset.path, asset=asset, version=version
)
except IntegrityError as e:
from dandiapi.api.services.asset.exceptions import AssetAlreadyExists
from dandiapi.api.services.asset.exceptions import AssetAlreadyExistsError

# If there are simultaneous requests to create the same asset, this check constraint can
# fail, and should be handled directly, rather than be allowed to bubble up
if 'unique-version-path' in str(e):
raise AssetAlreadyExists
raise AssetAlreadyExistsError

# Re-raise original exception otherwise
raise
Expand Down
30 changes: 15 additions & 15 deletions dandiapi/api/services/asset/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@
from dandiapi.api.models.asset import Asset, AssetBlob, EmbargoedAssetBlob
from dandiapi.api.models.version import Version
from dandiapi.api.services.asset.exceptions import (
AssetAlreadyExists,
AssetPathConflict,
DandisetOwnerRequired,
DraftDandisetNotModifiable,
ZarrArchiveBelongsToDifferentDandiset,
AssetAlreadyExistsError,
AssetPathConflictError,
DandisetOwnerRequiredError,
DraftDandisetNotModifiableError,
ZarrArchiveBelongsToDifferentDandisetError,
)
from dandiapi.zarr.models import ZarrArchive

Expand Down Expand Up @@ -58,9 +58,9 @@ def change_asset(
assert 'path' in new_metadata, 'Path must be present in new_metadata'

if not user.has_perm('owner', version.dandiset):
raise DandisetOwnerRequired
raise DandisetOwnerRequiredError
elif version.version != 'draft':
raise DraftDandisetNotModifiable
raise DraftDandisetNotModifiableError

path = new_metadata['path']
new_metadata_stripped = Asset.strip_metadata(new_metadata)
Expand All @@ -75,7 +75,7 @@ def change_asset(

# Verify we aren't changing path to the same value as an existing asset
if version.assets.filter(path=path).exclude(asset_id=asset.asset_id).exists():
raise AssetAlreadyExists
raise AssetAlreadyExistsError

with transaction.atomic():
remove_asset_from_version(user=user, asset=asset, version=version)
Expand Down Expand Up @@ -109,23 +109,23 @@ def add_asset_to_version(
assert 'path' in metadata, 'Path must be present in metadata'

if not user.has_perm('owner', version.dandiset):
raise DandisetOwnerRequired
raise DandisetOwnerRequiredError
elif version.version != 'draft':
raise DraftDandisetNotModifiable
raise DraftDandisetNotModifiableError

# Check if there are already any assets with the same path
path = metadata['path']
if version.assets.filter(path=path).exists():
raise AssetAlreadyExists
raise AssetAlreadyExistsError

# Check if there are any assets that conflict with this path
conflicts = get_conflicting_paths(path, version)
if conflicts:
raise AssetPathConflict(new_path=path, existing_paths=conflicts)
raise AssetPathConflictError(new_path=path, existing_paths=conflicts)

# Ensure zarr archive doesn't already belong to a dandiset
if zarr_archive and zarr_archive.dandiset != version.dandiset:
raise ZarrArchiveBelongsToDifferentDandiset
raise ZarrArchiveBelongsToDifferentDandisetError

if isinstance(asset_blob, EmbargoedAssetBlob):
embargoed_asset_blob = asset_blob
Expand Down Expand Up @@ -155,9 +155,9 @@ def add_asset_to_version(

def remove_asset_from_version(*, user, asset: Asset, version: Version) -> Version:
if not user.has_perm('owner', version.dandiset):
raise DandisetOwnerRequired
raise DandisetOwnerRequiredError
elif version.version != 'draft':
raise DraftDandisetNotModifiable
raise DraftDandisetNotModifiableError

with transaction.atomic():
# Remove asset paths and asset itself from version
Expand Down
12 changes: 6 additions & 6 deletions dandiapi/api/services/asset/exceptions.py
Original file line number Diff line number Diff line change
@@ -1,30 +1,30 @@
from rest_framework import status

from dandiapi.api.services.exceptions import DandiException
from dandiapi.api.services.exceptions import DandiError


class DandisetOwnerRequired(DandiException):
class DandisetOwnerRequiredError(DandiError):
http_status_code = status.HTTP_403_FORBIDDEN


class DraftDandisetNotModifiable(DandiException):
class DraftDandisetNotModifiableError(DandiError):
http_status_code = status.HTTP_405_METHOD_NOT_ALLOWED
message = 'Only draft versions can be modified.'


class AssetAlreadyExists(DandiException):
class AssetAlreadyExistsError(DandiError):
http_status_code = status.HTTP_409_CONFLICT
message = 'An asset with that path already exists'


class AssetPathConflict(DandiException):
class AssetPathConflictError(DandiError):
http_status_code = status.HTTP_409_CONFLICT

def __init__(self, new_path: str, existing_paths: list[str]) -> None:
message = f'Path of new asset "{new_path}" conflicts with existing assets: {existing_paths}'
super().__init__(message)


class ZarrArchiveBelongsToDifferentDandiset(DandiException):
class ZarrArchiveBelongsToDifferentDandisetError(DandiError):
http_status_code = status.HTTP_400_BAD_REQUEST
message = 'The zarr archive belongs to a different dandiset'
14 changes: 7 additions & 7 deletions dandiapi/api/services/dandiset/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

from dandiapi.api.models.dandiset import Dandiset
from dandiapi.api.models.version import Version
from dandiapi.api.services.dandiset.exceptions import DandisetAlreadyExists
from dandiapi.api.services.exceptions import AdminOnlyOperation, NotAllowed
from dandiapi.api.services.dandiset.exceptions import DandisetAlreadyExistsError
from dandiapi.api.services.exceptions import AdminOnlyOperationError, NotAllowedError
from dandiapi.api.services.version.metadata import _normalize_version_metadata


Expand All @@ -16,13 +16,13 @@ def create_dandiset(
version_metadata: dict,
) -> tuple[Dandiset, Version]:
if identifier and not user.is_superuser:
raise AdminOnlyOperation(
raise AdminOnlyOperationError(
'Creating a dandiset for a given identifier is an admin only operation.'
)

existing_dandiset = Dandiset.objects.filter(id=identifier).first()
if existing_dandiset:
raise DandisetAlreadyExists(f'Dandiset {existing_dandiset.identifier} already exists')
raise DandisetAlreadyExistsError(f'Dandiset {existing_dandiset.identifier} already exists')

embargo_status = Dandiset.EmbargoStatus.EMBARGOED if embargo else Dandiset.EmbargoStatus.OPEN
version_metadata = _normalize_version_metadata(
Expand All @@ -48,11 +48,11 @@ def create_dandiset(

def delete_dandiset(*, user, dandiset: Dandiset) -> None:
if not user.has_perm('owner', dandiset):
raise NotAllowed('Cannot delete dandisets which you do not own.')
raise NotAllowedError('Cannot delete dandisets which you do not own.')
if dandiset.versions.exclude(version='draft').exists():
raise NotAllowed('Cannot delete dandisets with published versions.')
raise NotAllowedError('Cannot delete dandisets with published versions.')
if dandiset.versions.filter(status=Version.Status.PUBLISHING).exists():
raise NotAllowed('Cannot delete dandisets that are currently being published.')
raise NotAllowedError('Cannot delete dandisets that are currently being published.')

# Delete all versions first, so that AssetPath deletion is cascaded
# through versions, rather than through zarrs directly
Expand Down
4 changes: 2 additions & 2 deletions dandiapi/api/services/dandiset/exceptions.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from rest_framework import status

from dandiapi.api.services.exceptions import DandiException
from dandiapi.api.services.exceptions import DandiError


class DandisetAlreadyExists(DandiException):
class DandisetAlreadyExistsError(DandiError):
http_status_code = status.HTTP_400_BAD_REQUEST
10 changes: 5 additions & 5 deletions dandiapi/api/services/embargo/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,16 @@

from dandiapi.api.copy import copy_object_multipart
from dandiapi.api.models import Asset, AssetBlob, Dandiset, Upload, Version
from dandiapi.api.services.asset.exceptions import DandisetOwnerRequired
from dandiapi.api.services.asset.exceptions import DandisetOwnerRequiredError
from dandiapi.api.tasks import unembargo_dandiset_task

from .exceptions import AssetNotEmbargoed, DandisetNotEmbargoed
from .exceptions import AssetNotEmbargoedError, DandisetNotEmbargoedError


def _unembargo_asset(asset: Asset):
"""Unembargo an asset by copying its blob to the public bucket."""
if asset.embargoed_blob is None:
raise AssetNotEmbargoed
raise AssetNotEmbargoedError

# Use existing AssetBlob if possible
etag = asset.embargoed_blob.etag
Expand Down Expand Up @@ -72,9 +72,9 @@ def _unembargo_dandiset(dandiset: Dandiset):
def unembargo_dandiset(*, user: User, dandiset: Dandiset):
"""Unembargo a dandiset by copying all embargoed asset blobs to the public bucket."""
if dandiset.embargo_status != Dandiset.EmbargoStatus.EMBARGOED:
raise DandisetNotEmbargoed
raise DandisetNotEmbargoedError

if not user.has_perm('owner', dandiset):
raise DandisetOwnerRequired
raise DandisetOwnerRequiredError

unembargo_dandiset_task.delay(dandiset.id)
6 changes: 3 additions & 3 deletions dandiapi/api/services/embargo/exceptions.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
from rest_framework import status

from dandiapi.api.services.exceptions import DandiException
from dandiapi.api.services.exceptions import DandiError


class AssetNotEmbargoed(DandiException):
class AssetNotEmbargoedError(DandiError):
http_status_code = status.HTTP_400_BAD_REQUEST
message = 'Only embargoed assets can be unembargoed.'


class DandisetNotEmbargoed(DandiException):
class DandisetNotEmbargoedError(DandiError):
http_status_code = status.HTTP_400_BAD_REQUEST
message = 'Dandiset not embargoed'
6 changes: 3 additions & 3 deletions dandiapi/api/services/exceptions.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from rest_framework import status


class DandiException(Exception):
class DandiError(Exception):
message: str | None
http_status_code: int | None

Expand All @@ -14,10 +14,10 @@ def __init__(
super().__init__(*args)


class NotAllowed(DandiException):
class NotAllowedError(DandiError):
message = 'Action not allowed'
http_status_code = status.HTTP_403_FORBIDDEN


class AdminOnlyOperation(DandiException):
class AdminOnlyOperationError(DandiError):
pass
11 changes: 7 additions & 4 deletions dandiapi/api/services/metadata/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@
import jsonschema.exceptions

from dandiapi.api.models import Asset, Version
from dandiapi.api.services.metadata.exceptions import AssetHasBeenPublished, VersionHasBeenPublished
from dandiapi.api.services.metadata.exceptions import (
AssetHasBeenPublishedError,
VersionHasBeenPublishedError,
)
from dandiapi.api.services.publish import _build_publishable_version_from_draft

logger = get_task_logger(__name__)
Expand Down Expand Up @@ -38,7 +41,7 @@ def validate_asset_metadata(*, asset: Asset) -> bool:

# Published assets are immutable
if asset.published:
raise AssetHasBeenPublished
raise AssetHasBeenPublishedError

# track the state of the asset before to use optimistic locking
asset_state = asset.status
Expand Down Expand Up @@ -76,7 +79,7 @@ def validate_asset_metadata(*, asset: Asset) -> bool:

def version_aggregate_assets_summary(version: Version) -> None:
if version.version != 'draft':
raise VersionHasBeenPublished
raise VersionHasBeenPublishedError

version.metadata['assetsSummary'] = aggregate_assets_summary(
asset.full_metadata
Expand Down Expand Up @@ -117,7 +120,7 @@ def _build_validatable_version_metadata(version: Version) -> dict:

# Published versions are immutable
if version.version != 'draft':
raise VersionHasBeenPublished
raise VersionHasBeenPublishedError

with transaction.atomic():
# validating version metadata needs to lock the version to avoid racing with
Expand Down
6 changes: 3 additions & 3 deletions dandiapi/api/services/metadata/exceptions.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
from rest_framework import status

from dandiapi.api.services.exceptions import DandiException
from dandiapi.api.services.exceptions import DandiError


class AssetHasBeenPublished(DandiException):
class AssetHasBeenPublishedError(DandiError):
http_status_code = status.HTTP_500_INTERNAL_SERVER_ERROR
message = 'This asset has been published and cannot be modified.'


class VersionHasBeenPublished(DandiException):
class VersionHasBeenPublishedError(DandiError):
http_status_code = status.HTTP_500_INTERNAL_SERVER_ERROR
message = 'This version has been published and cannot be modified.'
32 changes: 16 additions & 16 deletions dandiapi/api/services/publish/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,14 @@
from dandiapi.api import doi
from dandiapi.api.asset_paths import add_version_asset_paths
from dandiapi.api.models import Asset, Dandiset, Version
from dandiapi.api.services.exceptions import NotAllowed
from dandiapi.api.services.exceptions import NotAllowedError
from dandiapi.api.services.publish.exceptions import (
DandisetAlreadyPublished,
DandisetAlreadyPublishing,
DandisetBeingValidated,
DandisetInvalidMetadata,
DandisetNotLocked,
DandisetValidationPending,
DandisetAlreadyPublishedError,
DandisetAlreadyPublishingError,
DandisetBeingValidatedError,
DandisetInvalidMetadataError,
DandisetNotLockedError,
DandisetValidationPendingError,
)
from dandiapi.api.tasks import write_manifest_files

Expand Down Expand Up @@ -45,27 +45,27 @@ def _lock_dandiset_for_publishing(*, user: User, dandiset: Dandiset) -> None:
not user.has_perm('owner', dandiset)
or dandiset.embargo_status != Dandiset.EmbargoStatus.OPEN
):
raise NotAllowed
raise NotAllowedError
if dandiset.zarr_archives.exists() or dandiset.embargoed_zarr_archives.exists():
# TODO: return a string instead of a list here
raise NotAllowed(['Cannot publish dandisets which contain zarrs'], 400) # type: ignore
raise NotAllowedError(['Cannot publish dandisets which contain zarrs'], 400) # type: ignore

with transaction.atomic():
draft_version: Version = dandiset.versions.select_for_update().get(version='draft')
if not draft_version.publishable:
match draft_version.status:
case Version.Status.PUBLISHED:
raise DandisetAlreadyPublished
raise DandisetAlreadyPublishedError
case Version.Status.PUBLISHING:
raise DandisetAlreadyPublishing
raise DandisetAlreadyPublishingError
case Version.Status.VALIDATING:
raise DandisetBeingValidated
raise DandisetBeingValidatedError
case Version.Status.INVALID:
raise DandisetInvalidMetadata
raise DandisetInvalidMetadataError
case Version.Status.PENDING:
raise DandisetValidationPending
raise DandisetValidationPendingError
case Version.Status.VALID:
raise DandisetInvalidMetadata
raise DandisetInvalidMetadataError
case other:
raise NotImplementedError(
f'Draft version of dandiset {dandiset.identifier} '
Expand Down Expand Up @@ -110,7 +110,7 @@ def _publish_dandiset(dandiset_id: int) -> None:
)

if old_version.status != Version.Status.PUBLISHING:
raise DandisetNotLocked(
raise DandisetNotLockedError(
'Dandiset must be in PUBLISHING state. Call `_lock_dandiset_for_publishing()` '
'before this function.'
)
Expand Down
Loading

0 comments on commit 63cc75e

Please sign in to comment.