Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Misc. Django/Python improvements #1050

Merged
merged 3 commits into from
Apr 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions dandiapi/api/dashboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from django.contrib.auth.mixins import LoginRequiredMixin, UserPassesTestMixin
from django.contrib.auth.models import User
from django.core.exceptions import PermissionDenied
from django.db.models import Exists, OuterRef, Q
from django.db.models import Exists, OuterRef
from django.shortcuts import get_object_or_404, render
from django.views.decorators.http import require_http_methods
from django.views.generic.base import TemplateView
Expand Down Expand Up @@ -60,7 +60,7 @@ def _non_valid_assets(self):
has_version=Exists(Version.objects.filter(assets=OuterRef('id')))
)
.filter(has_version=True)
.filter(~Q(status=Asset.Status.VALID))
.exclude(status=Asset.Status.VALID)
)

def _uploads(self):
Expand Down
9 changes: 5 additions & 4 deletions dandiapi/api/mail.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from __future__ import annotations

import logging
from typing import Optional, Union

from allauth.socialaccount.models import SocialAccount
from django.conf import settings
Expand All @@ -19,7 +20,7 @@
ADMIN_EMAIL = 'info@dandiarchive.org'


def build_message(subject: str, message: str, to: list[str], html_message: Optional[str] = None):
def build_message(subject: str, message: str, to: list[str], html_message: str | None = None):
message = mail.EmailMultiAlternatives(subject=subject, body=message, to=to)
if html_message is not None:
message.attach_alternative(html_message, 'text/html')
Expand Down Expand Up @@ -165,7 +166,7 @@ def send_rejected_user_message(user: User, socialaccount: SocialAccount):
connection.send_messages(messages)


def build_pending_users_message(users: Union[QuerySet, list[User]]):
def build_pending_users_message(users: QuerySet | list[User]):
render_context = {**BASE_RENDER_CONTEXT, 'users': users}
return build_message(
subject='DANDI: new user registrations to review',
Expand All @@ -174,7 +175,7 @@ def build_pending_users_message(users: Union[QuerySet, list[User]]):
)


def send_pending_users_message(users: Union[QuerySet, list[User]]):
def send_pending_users_message(users: QuerySet | list[User]):
logger.info(f'Sending pending users message to admins at {ADMIN_EMAIL}')
messages = [build_pending_users_message(users)]
with mail.get_connection() as connection:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
from pprint import pformat

from dandischema import migrate
from django.db.models import Q
import djclick as click

from dandiapi.api.models import Version
Expand All @@ -17,7 +16,7 @@ def migrate_published_version_metadata(dandiset: str, published_version: str, to
click.echo(
f'Migrating published version {dandiset}/{published_version} metadata to version {to_version}' # noqa: E501
)
version = Version.objects.filter(~Q(version='draft')).get(
version = Version.objects.exclude(version='draft').get(
dandiset=dandiset, version=published_version
)
metadata = version.metadata
Expand Down
4 changes: 1 addition & 3 deletions dandiapi/api/models/dandiset.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
from __future__ import annotations

from typing import Optional

from django.db import models
from django_extensions.db.models import TimeStampedModel
from guardian.shortcuts import assign_perm, get_objects_for_user, get_users_with_perms, remove_perm
Expand Down Expand Up @@ -55,7 +53,7 @@ class Meta:
permissions = [('owner', 'Owns the dandiset')]

@property
def identifier(self) -> Optional[str]:
def identifier(self) -> str | None:
# Compare against None, to allow id 0
return f'{self.id:06}' if self.id is not None else ''

Expand Down
4 changes: 2 additions & 2 deletions dandiapi/api/models/version.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ def valid(self) -> bool:
from .asset import Asset

# Return False if any asset is not VALID
return not self.assets.filter(~models.Q(status=Asset.Status.VALID)).exists()
return not self.assets.exclude(status=Asset.Status.VALID).exists()

@property
def publish_status(self) -> Version.Status:
Expand All @@ -81,7 +81,7 @@ def publish_status(self) -> Version.Status:
# Import here to avoid dependency cycle
from .asset import Asset

invalid_asset = self.assets.filter(~models.Q(status=Asset.Status.VALID)).first()
invalid_asset = self.assets.exclude(status=Asset.Status.VALID).first()
if invalid_asset:
return Version.Status.INVALID

Expand Down
3 changes: 1 addition & 2 deletions dandiapi/api/models/zarr.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
from datetime import timedelta
import logging
from pathlib import Path
from typing import Optional
from urllib.parse import urlparse, urlunparse
from uuid import uuid4

Expand Down Expand Up @@ -193,7 +192,7 @@ def upload_in_progress(self) -> bool:
return self.active_uploads.exists()

@property
def checksum(self) -> Optional[str]:
def checksum(self) -> str | None:
try:
return self.get_checksum()
except ValidationError:
Expand Down
7 changes: 4 additions & 3 deletions dandiapi/api/tasks/zarr.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from __future__ import annotations

from datetime import datetime
from pathlib import Path
from typing import Optional

import boto3
from celery import shared_task
Expand Down Expand Up @@ -43,7 +44,7 @@ def get_client():
return storage.connection.meta.client


def yield_files(bucket: str, prefix: Optional[str] = None):
def yield_files(bucket: str, prefix: str | None = None):
"""Get all objects in the bucket, through repeated object listing."""
client = get_client()
common_options = {'Bucket': bucket}
Expand Down Expand Up @@ -98,7 +99,7 @@ def __init__(self, *args, session_start: datetime, **kwargs):
# Set the start of when new files may have been written
self._session_start = session_start

def read_checksum_file(self) -> Optional[ZarrChecksumListing]:
def read_checksum_file(self) -> ZarrChecksumListing | None:
"""Load a checksum listing from the checksum file."""
storage = self.zarr_archive.storage
checksum_path = self.checksum_file_path
Expand Down
6 changes: 3 additions & 3 deletions dandiapi/api/views/asset.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,9 @@
from urllib.parse import urlencode

from django.core.paginator import EmptyPage, Page, Paginator
from django.db import models, transaction
from django.db import transaction
from django.http import HttpResponseRedirect
from django.http.response import Http404
from django.shortcuts import get_object_or_404
from django.urls import reverse
from django.utils.decorators import method_decorator
from django_filters import rest_framework as filters
Expand All @@ -29,6 +28,7 @@
from rest_framework import serializers, status
from rest_framework.decorators import action
from rest_framework.exceptions import NotAuthenticated, PermissionDenied, ValidationError
from rest_framework.generics import get_object_or_404
from rest_framework.response import Response
from rest_framework.viewsets import GenericViewSet, ReadOnlyModelViewSet
from rest_framework_extensions.mixins import DetailSerializerMixin, NestedViewSetMixin
Expand Down Expand Up @@ -450,7 +450,7 @@ def update(self, request, versions__dandiset__pk, versions__version, **kwargs):
# Verify we aren't changing path to the same value as an existing asset
if (
version.assets.filter(path=new_asset.path)
.filter(~models.Q(asset_id=old_asset.asset_id))
.exclude(asset_id=old_asset.asset_id)
.exists()
):
return Response(
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 @@ -5,10 +5,9 @@
from django.conf import settings
from django.contrib.auth.models import User
from django.core.exceptions import ObjectDoesNotExist
from django.db.models import Count, OuterRef, Q, Subquery, Sum
from django.db.models import Count, OuterRef, Subquery, Sum
from django.db.utils import IntegrityError
from django.http import Http404
from django.shortcuts import get_object_or_404
from django.utils.decorators import method_decorator
from drf_yasg.utils import no_body, swagger_auto_schema
from guardian.decorators import permission_required_or_403
Expand All @@ -17,6 +16,7 @@
from rest_framework import filters, status
from rest_framework.decorators import action
from rest_framework.exceptions import NotAuthenticated, PermissionDenied
from rest_framework.generics import get_object_or_404
from rest_framework.request import Request
from rest_framework.response import Response
from rest_framework.serializers import ValidationError
Expand Down Expand Up @@ -270,7 +270,7 @@ def destroy(self, request, dandiset__pk):
"""
dandiset: Dandiset = self.get_object()

if dandiset.versions.filter(~Q(version='draft')).exists():
if dandiset.versions.exclude(version='draft').exists():
return Response(
'Cannot delete dandisets with published versions.',
status=status.HTTP_403_FORBIDDEN,
Expand Down
2 changes: 1 addition & 1 deletion dandiapi/api/views/version.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
from django.db import transaction
from django.shortcuts import get_object_or_404
from django.utils.decorators import method_decorator
from drf_yasg.utils import no_body, swagger_auto_schema
from guardian.decorators import permission_required_or_403
from rest_framework import status
from rest_framework.decorators import action
from rest_framework.exceptions import NotAuthenticated, PermissionDenied, ValidationError
from rest_framework.generics import get_object_or_404
from rest_framework.response import Response
from rest_framework.viewsets import ReadOnlyModelViewSet
from rest_framework_extensions.mixins import DetailSerializerMixin, NestedViewSetMixin
Expand Down
2 changes: 1 addition & 1 deletion dandiapi/api/views/zarr.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@
from django.core.exceptions import PermissionDenied
from django.db import IntegrityError, transaction
from django.http.response import HttpResponseRedirect
from django.shortcuts import get_object_or_404
from django.urls import reverse
from drf_yasg import openapi
from drf_yasg.utils import no_body, swagger_auto_schema
from minio_storage.storage import MinioStorage
from rest_framework import serializers, status
from rest_framework.decorators import action, api_view
from rest_framework.exceptions import ValidationError
from rest_framework.generics import get_object_or_404
from rest_framework.response import Response
from rest_framework.viewsets import ReadOnlyModelViewSet
from storages.backends.s3boto3 import S3Boto3Storage
Expand Down
4 changes: 2 additions & 2 deletions dandiapi/api/zarr_checksums.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from contextlib import AbstractContextManager
from dataclasses import dataclass, field
from pathlib import Path
from typing import TYPE_CHECKING, Optional
from typing import TYPE_CHECKING

from django.conf import settings
from django.core.files.base import ContentFile
Expand Down Expand Up @@ -75,7 +75,7 @@ def checksum_listing(self) -> ZarrChecksumListing:
raise ValueError('This method is only valid when used by a context manager')
return self._serializer.generate_listing(self._checksums)

def read_checksum_file(self) -> Optional[ZarrChecksumListing]:
def read_checksum_file(self) -> ZarrChecksumListing | None:
"""Load a checksum listing from the checksum file."""
storage = self.zarr_archive.storage
checksum_path = self.checksum_file_path
Expand Down