Skip to content

Commit

Permalink
Pre-release cleanup pass (#301)
Browse files Browse the repository at this point in the history
* Adds missing kwarg check to clean command

* Spelling fixes in autocomplete command

* Updates packaging

* Drops unused celery check

* Remove redundant code

* Auto resolve base names
  • Loading branch information
djperrefort authored May 14, 2024
1 parent 0a71d96 commit 6aefad2
Show file tree
Hide file tree
Showing 17 changed files with 67 additions and 173 deletions.
10 changes: 5 additions & 5 deletions keystone_api/apps/admin_utils/management/commands/clean.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
|------------|------------------------------------------------------------------|
| --static | Delete the static root directory |
| --uploads | Delete all user uploaded file data |
| --db | Delete all sqlite database files |
| --sqlite | Delete all sqlite database files |
| --all | Shorthand for deleting everything |
"""

Expand All @@ -33,18 +33,18 @@ def add_arguments(self, parser: ArgumentParser) -> None:
group = parser.add_argument_group('clean options')
group.add_argument('--static', action='store_true', help='Delete the static root directory')
group.add_argument('--uploads', action='store_true', help='Delete all user uploaded file data')
group.add_argument('--db', action='store_true', help='Delete all sqlite database files')
group.add_argument('--sqlite', action='store_true', help='Delete all sqlite database files')
group.add_argument('--all', action='store_true', help='Shorthand for deleting all targets')

def handle(self, *args, **options) -> None:
"""Handle the command execution.
"""Handle the command execution
Args:
*args: Additional positional arguments.
**options: Additional keyword arguments.
"""

if not any([options['static'], options['db'], options['all']]):
if not any([options['static'], options['uploads'], options['sqlite'], options['all']]):
self.stderr.write('At least one deletion target is required. `See clean --help` for details.')

if options['static'] or options['all']:
Expand All @@ -55,7 +55,7 @@ def handle(self, *args, **options) -> None:
self.stdout.write(self.style.SUCCESS('Removing user uploads...'))
shutil.rmtree(settings.MEDIA_ROOT, ignore_errors=True)

if options['db'] or options['all']:
if options['sqlite'] or options['all']:
self.stdout.write(self.style.SUCCESS('Removing sqlite files...'))
for db_settings in settings.DATABASES.values():
if 'sqlite' in db_settings['ENGINE']:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def _handle(self) -> None:
# Find the user's .bash_prf le or .bashrc file
profile_path = self.get_profile_path()
if profile_path is None:
self.stderr.write(f'No .bash_profile or .bashrc fie found.')
self.stderr.write(f'No .bash_profile or .bashrc file found.')
exit(1)

# Copy the completion script into the user's home directory
Expand All @@ -58,15 +58,15 @@ def prompt_for_confirmation() -> bool:
print(
'This command will make the following changes:\n',
' - A file `.keystone_autocomplete` will be add to your home directory\n'
' - A line of setup code will be added to your `.bash_profile`\n'
' - A line of setup code will be added to your .bash_profile or .bashrc file`\n'
)

while True:
answer = input('Do you want to continue? [Y/n]: ').lower()
if answer in ('y', ''):
answer = input('Do you want to continue? [y/N]: ').lower()
if answer == 'y':
return True

elif answer == 'n':
elif answer in ('n', ''):
return False

print('Unrecognized input.')
Expand Down
65 changes: 0 additions & 65 deletions keystone_api/apps/allocations/managers.py

This file was deleted.

7 changes: 0 additions & 7 deletions keystone_api/apps/allocations/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
from django.template.defaultfilters import truncatechars

from apps.users.models import ResearchGroup, User
from .managers import *

__all__ = [
'Allocation',
Expand Down Expand Up @@ -45,8 +44,6 @@ class Allocation(RGModelInterface, models.Model):
cluster: Cluster = models.ForeignKey('Cluster', on_delete=models.CASCADE)
request: AllocationRequest = models.ForeignKey('AllocationRequest', on_delete=models.CASCADE)

objects = AllocationManager()

def get_research_group(self) -> ResearchGroup:
"""Return the research group tied to the current record"""

Expand Down Expand Up @@ -78,8 +75,6 @@ class StatusChoices(models.TextChoices):

group: ResearchGroup = models.ForeignKey(ResearchGroup, on_delete=models.CASCADE)

objects = AllocationRequestManager()

def clean(self) -> None:
"""Validate the model instance
Expand Down Expand Up @@ -119,8 +114,6 @@ class StatusChoices(models.TextChoices):
request: AllocationRequest = models.ForeignKey(AllocationRequest, on_delete=models.CASCADE)
reviewer: User = models.ForeignKey(User, on_delete=models.CASCADE)

objects = AllocationRequestReviewManager()

def get_research_group(self) -> ResearchGroup:
"""Return the research group tied to the current record"""

Expand Down
8 changes: 4 additions & 4 deletions keystone_api/apps/allocations/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@
app_name = 'allocations'

router = DefaultRouter()
router.register(r'clusters', ClusterViewSet, basename='Cluster')
router.register(r'allocations', AllocationViewSet, basename='Allocation')
router.register(r'requests', AllocationRequestViewSet, basename='AllocationRequest')
router.register(r'reviews', AllocationRequestReviewViewSet, basename='AllocationRequestReview')
router.register(r'clusters', ClusterViewSet)
router.register(r'allocations', AllocationViewSet)
router.register(r'requests', AllocationRequestViewSet)
router.register(r'reviews', AllocationRequestReviewViewSet)

urlpatterns = router.urls
28 changes: 18 additions & 10 deletions keystone_api/apps/allocations/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,50 +18,58 @@
'ClusterViewSet'
]

from ..users.models import ResearchGroup


class AllocationViewSet(viewsets.ModelViewSet):
"""Manage allocations for user research groups."""

permission_classes = [permissions.IsAuthenticated, StaffWriteGroupRead]
queryset = Allocation.objects.all()
serializer_class = AllocationSerializer
permission_classes = [permissions.IsAuthenticated, StaffWriteGroupRead]

def get_queryset(self) -> list[Allocation]:
"""Return a list of allocations for the currently authenticated user"""

if self.request.user.is_staff or self.request.user.is_superuser:
return Allocation.objects.all()
return self.queryset

return Allocation.objects.affiliated_with_user(self.request.user).all()
research_groups = ResearchGroup.objects.groups_for_user(self.request.user)
return Allocation.objects.filter(request__group__in=research_groups)


class AllocationRequestViewSet(viewsets.ModelViewSet):
"""Manage allocation requests submitted by user research groups."""

permission_classes = [permissions.IsAuthenticated, GroupAdminCreateGroupRead]
queryset = AllocationRequest.objects.all()
serializer_class = AllocationRequestSerializer
permission_classes = [permissions.IsAuthenticated, GroupAdminCreateGroupRead]

def get_queryset(self) -> list[AllocationRequest]:
"""Return a list of allocation requests for the currently authenticated user"""

if self.request.user.is_staff or self.request.user.is_superuser:
return AllocationRequest.objects.all()
return self.queryset

return AllocationRequest.objects.affiliated_with_user(self.request.user).all()
research_groups = ResearchGroup.objects.groups_for_user(self.request.user)
return AllocationRequest.objects.filter(group__in=research_groups)


class AllocationRequestReviewViewSet(viewsets.ModelViewSet):
"""Manage reviews of allocation request submitted by administrators."""

permission_classes = [permissions.IsAuthenticated, StaffWriteGroupRead]
queryset = AllocationRequestReview.objects.all()
serializer_class = AllocationRequestReviewSerializer
permission_classes = [permissions.IsAuthenticated, StaffWriteGroupRead]

def get_queryset(self) -> list[Allocation]:
"""Return a list of allocation reviews for the currently authenticated user"""

if self.request.user.is_staff or self.request.user.is_superuser:
return AllocationRequestReview.objects.all()
return self.queryset

return AllocationRequestReview.objects.affiliated_with_user(self.request.user).all()
research_groups = ResearchGroup.objects.groups_for_user(self.request.user)
return AllocationRequestReview.objects.filter(request__group__in=research_groups)

def create(self, request, *args, **kwargs) -> Response:
"""Create a new `AllocationRequestReview` object"""
Expand All @@ -81,5 +89,5 @@ class ClusterViewSet(viewsets.ModelViewSet):
"""Configuration settings for managed Slurm clusters."""

queryset = Cluster.objects.all()
permission_classes = [permissions.IsAuthenticated, StaffWriteAuthenticatedRead]
serializer_class = ClusterSerializer
permission_classes = [permissions.IsAuthenticated, StaffWriteAuthenticatedRead]
4 changes: 2 additions & 2 deletions keystone_api/apps/research_products/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
app_name = 'research_products'

router = DefaultRouter()
router.register(r'publications', PublicationViewSet, basename='publication')
router.register(r'grants', GrantViewSet, basename='grant')
router.register(r'publications', PublicationViewSet)
router.register(r'grants', GrantViewSet)

urlpatterns = router.urls
16 changes: 12 additions & 4 deletions keystone_api/apps/research_products/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,28 +16,36 @@
class PublicationViewSet(viewsets.ModelViewSet):
"""Manage metadata for research publications."""

permission_classes = [permissions.IsAuthenticated, permissions.IsAdminUser | GroupMemberAll]
queryset = Publication.objects.all()
serializer_class = PublicationSerializer
permission_classes = [
permissions.IsAuthenticated,
permissions.IsAdminUser | GroupMemberAll
]

def get_queryset(self) -> list[Publication]:
"""Return a list of allocation requests for the currently authenticated user"""

if self.request.user.is_staff:
return Publication.objects.all()
return self.queryset

return Publication.objects.affiliated_with_user(self.request.user).all()


class GrantViewSet(viewsets.ModelViewSet):
"""Track funding awards and grant information."""

permission_classes = [permissions.IsAuthenticated, permissions.IsAdminUser | GroupMemberReadGroupAdminWrite]
queryset = Grant.objects.all()
serializer_class = GrantSerializer
permission_classes = [
permissions.IsAuthenticated,
permissions.IsAdminUser | GroupMemberReadGroupAdminWrite
]

def get_queryset(self) -> list[Grant]:
"""Return a list of allocation requests for the currently authenticated user"""

if self.request.user.is_staff:
return Grant.objects.all()
return self.queryset

return Grant.objects.affiliated_with_user(self.request.user).all()
22 changes: 0 additions & 22 deletions keystone_api/apps/scheduler/apps.py

This file was deleted.

32 changes: 0 additions & 32 deletions keystone_api/apps/scheduler/checks.py

This file was deleted.

8 changes: 4 additions & 4 deletions keystone_api/apps/users/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@
class User(auth_models.AbstractBaseUser, auth_models.PermissionsMixin):
"""Proxy model for the built-in django `User` model"""

USERNAME_FIELD = 'username'
EMAIL_FIELD = "email"
REQUIRED_FIELDS = ['email', 'first_name', 'last_name']

username = models.CharField('username', max_length=150, unique=True, validators=[UnicodeUsernameValidator()])
first_name = models.CharField('first name', max_length=150)
last_name = models.CharField('last name', max_length=150)
Expand All @@ -32,10 +36,6 @@ class User(auth_models.AbstractBaseUser, auth_models.PermissionsMixin):

objects = UserManager()

USERNAME_FIELD = 'username'
EMAIL_FIELD = "email"
REQUIRED_FIELDS = ['email', 'first_name', 'last_name']


class ResearchGroup(models.Model):
"""A user research group tied to a slurm account"""
Expand Down
2 changes: 1 addition & 1 deletion keystone_api/apps/users/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,4 @@ def has_permission(self, request, view) -> bool:
if request.method in permissions.SAFE_METHODS:
return request.user.is_authenticated

return request.user.is_staff or request.user.is_superuser
return request.user.is_staff
Loading

0 comments on commit 6aefad2

Please sign in to comment.