Skip to content

Commit

Permalink
Merge pull request #1752 from dandi/else-return
Browse files Browse the repository at this point in the history
  • Loading branch information
brianhelba authored Dec 5, 2023
2 parents 61a35a3 + 18a344d commit 445f61d
Show file tree
Hide file tree
Showing 12 changed files with 37 additions and 48 deletions.
5 changes: 2 additions & 3 deletions dandiapi/api/doi.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,8 @@ def delete_doi(doi: str) -> None:
if e.response and e.response.status_code == 404:
logging.warning('Tried to get data for nonexistent DOI %s', doi)
return
else:
logging.exception('Failed to fetch data for DOI %s', doi)
raise
logging.exception('Failed to fetch data for DOI %s', doi)
raise
if r.json()['data']['attributes']['state'] == 'draft':
try:
s.delete(doi_url).raise_for_status()
Expand Down
17 changes: 8 additions & 9 deletions dandiapi/api/mail.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,14 @@ def user_greeting_name(user: User, socialaccount: SocialAccount = None) -> str:

if socialaccount is None:
return user_to_dict(user)['name']
else:
social_user = social_account_to_dict(socialaccount)

# it's possible to have social users without a name if they've signed up, not filled out
# the questionnaire, and their github account has no attached name.
if social_user.get('name'):
return f'{social_user["name"]} (Github ID: {social_user["username"]})'
else:
return social_user['username']

social_user = social_account_to_dict(socialaccount)

# it's possible to have social users without a name if they've signed up, not filled out
# the questionnaire, and their github account has no attached name.
if social_user.get('name'):
return f'{social_user["name"]} (Github ID: {social_user["username"]})'
return social_user['username']


def build_message(subject: str, message: str, to: list[str], html_message: str | None = None):
Expand Down
20 changes: 8 additions & 12 deletions dandiapi/api/models/asset.py
Original file line number Diff line number Diff line change
Expand Up @@ -203,37 +203,33 @@ def is_zarr(self):
def size(self):
if self.is_blob:
return self.blob.size
elif self.is_embargoed_blob:
if self.is_embargoed_blob:
return self.embargoed_blob.size
else:
return self.zarr.size
return self.zarr.size

@property
def sha256(self):
if self.is_blob:
return self.blob.sha256
elif self.is_embargoed_blob:
if self.is_embargoed_blob:
return self.embargoed_blob.sha256
else:
raise RuntimeError('Zarr does not support SHA256')
raise RuntimeError('Zarr does not support SHA256')

@property
def digest(self) -> dict[str, str]:
if self.is_blob:
return self.blob.digest
elif self.is_embargoed_blob:
if self.is_embargoed_blob:
return self.embargoed_blob.digest
else:
return self.zarr.digest
return self.zarr.digest

@property
def s3_url(self) -> str:
if self.is_blob:
return self.blob.s3_url
elif self.is_embargoed_blob:
if self.is_embargoed_blob:
return self.embargoed_blob.s3_url
else:
return self.zarr.s3_url
return self.zarr.s3_url

def is_different_from(
self,
Expand Down
6 changes: 3 additions & 3 deletions dandiapi/api/services/asset/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ def change_asset(

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

path = new_metadata['path']
Expand Down Expand Up @@ -110,7 +110,7 @@ def add_asset_to_version(

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

# Check if there are already any assets with the same path
Expand Down Expand Up @@ -156,7 +156,7 @@ 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 DandisetOwnerRequiredError
elif version.version != 'draft':
if version.version != 'draft':
raise DraftDandisetNotModifiableError

with transaction.atomic():
Expand Down
3 changes: 1 addition & 2 deletions dandiapi/api/storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,8 +194,7 @@ def etag_from_blob_name(self, blob_name) -> str | None:
except S3Error as e:
if e.code == 'NoSuchKey':
return None
else:
raise
raise
else:
return response.etag

Expand Down
5 changes: 2 additions & 3 deletions dandiapi/api/views/asset.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ def download(self, request, *args, **kwargs):
return HttpResponseRedirect(
storage.generate_presigned_download_url(asset_blob.blob.name, asset_basename)
)
elif content_disposition == 'inline':
if content_disposition == 'inline':
url = storage.generate_presigned_inline_url(
asset_blob.blob.name,
asset_basename,
Expand All @@ -167,8 +167,7 @@ def download(self, request, *args, **kwargs):
)

return HttpResponseRedirect(url)
else:
raise TypeError('Invalid content_disposition: %s', content_disposition)
raise TypeError('Invalid content_disposition: %s', content_disposition)

@swagger_auto_schema(
method='GET',
Expand Down
2 changes: 1 addition & 1 deletion dandiapi/api/views/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ def authorize_view(request: HttpRequest) -> HttpResponse:
f'{reverse("user-questionnaire")}'
f'?{request.META["QUERY_STRING"]}&QUESTIONS={json.dumps(NEW_USER_QUESTIONS)}'
)
elif not user.is_anonymous and (not user.first_name or not user.last_name):
if not user.is_anonymous and (not user.first_name or not user.last_name):
# if this user doesn't have a first/last name available, redirect them to a
# form to provide those before they can log in.
return HttpResponseRedirect(
Expand Down
6 changes: 3 additions & 3 deletions dandiapi/api/views/dandiset.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,14 @@ def filter_queryset(self, request, queryset, view):
# ordering can be either 'created' or '-created', so test for both
if ordering.endswith('id'):
return queryset.order_by(ordering)
elif ordering.endswith('name'):
if ordering.endswith('name'):
# name refers to the name of the most recent version, so a subquery is required
latest_version = Version.objects.filter(dandiset=OuterRef('pk')).order_by(
'-created'
)[:1]
queryset = queryset.annotate(name=Subquery(latest_version.values('metadata__name')))
return queryset.order_by(ordering)
elif ordering.endswith('modified'):
if ordering.endswith('modified'):
# modified refers to the modification timestamp of the most
# recent version, so a subquery is required
latest_version = Version.objects.filter(dandiset=OuterRef('pk')).order_by(
Expand All @@ -70,7 +70,7 @@ def filter_queryset(self, request, queryset, view):
modified_version=Subquery(latest_version.values('modified'))
)
return queryset.order_by(f'{ordering}_version')
elif ordering.endswith('size'):
if ordering.endswith('size'):
latest_version = Version.objects.filter(dandiset=OuterRef('pk')).order_by(
'-created'
)[:1]
Expand Down
2 changes: 1 addition & 1 deletion dandiapi/api/views/upload.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ def upload_initialize_view(request: Request) -> HttpResponseBase:
status=status.HTTP_409_CONFLICT,
headers={'Location': asset_blobs.first().blob_id},
)
elif dandiset.embargo_status != Dandiset.EmbargoStatus.OPEN:
if dandiset.embargo_status != Dandiset.EmbargoStatus.OPEN:
embargoed_asset_blobs = EmbargoedAssetBlob.objects.filter(dandiset=dandiset, etag=etag)
if embargoed_asset_blobs.exists():
return Response(
Expand Down
13 changes: 6 additions & 7 deletions dandiapi/api/views/version.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,14 +150,13 @@ def destroy(self, request, **kwargs):
'Cannot delete draft versions',
status=status.HTTP_403_FORBIDDEN,
)
elif not request.user.is_superuser:
if not request.user.is_superuser:
return Response(
'Cannot delete published versions',
status=status.HTTP_403_FORBIDDEN,
)
else:
doi = version.doi
version.delete()
if doi is not None:
delete_doi_task.delay(doi)
return Response(None, status=status.HTTP_204_NO_CONTENT)
doi = version.doi
version.delete()
if doi is not None:
delete_doi_task.delay(doi)
return Response(None, status=status.HTTP_204_NO_CONTENT)
3 changes: 1 addition & 2 deletions dandiapi/drf_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@ def rewrap_django_core_exceptions(exc: Exception, ctx: dict) -> Response | None:
# Dandi returns validation errors with 1 problem as a raw
# message. Support this for now, consider using the DRF enveloped format in the future.
return Response(exc.error_list[0].message, status=status.HTTP_400_BAD_REQUEST)
else:
exc = drf_exceptions.ValidationError(as_serializer_error(exc))
exc = drf_exceptions.ValidationError(as_serializer_error(exc))
elif isinstance(exc, DandiError):
return Response(exc.message, status=exc.http_status_code or status.HTTP_400_BAD_REQUEST)

Expand Down
3 changes: 1 addition & 2 deletions dandiapi/swagger.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,4 @@ def split_summary_from_description(self, description):
summary, description = super().split_summary_from_description(description)
if summary is None:
return description, None
else:
return summary, description
return summary, description

0 comments on commit 445f61d

Please sign in to comment.