Skip to content

Commit

Permalink
Adapt __str__ and __repr__ methods for DB
Browse files Browse the repository at this point in the history
This commit removes some debugging functionability in favor of production DB.
`__str__` and `__repr__` methods won't be so descriptive now since we are
removing some information from their rendering. This is because to render them
properly we need to hit the DB multiple times in the worst case --generating 500
on some user requests that need to be logged in Sentry/New Relic.

There are better ways for this: disabling logging on production and enabling it
on DEBUG + Django Shell, but that requires more extra work that doesn't seems
super priority right now. We can come back later and add them as we need them if
we want.

Closes #10954
  • Loading branch information
humitos committed May 16, 2024
1 parent a6130d3 commit 496f3c8
Show file tree
Hide file tree
Showing 19 changed files with 52 additions and 114 deletions.
3 changes: 0 additions & 3 deletions readthedocs/analytics/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,6 @@ class Meta:
),
]

def __str__(self):
return f"PageView: [{self.project.slug}] - {self.full_path or self.path} for {self.date}"

@classmethod
def top_viewed_pages(
cls, project, since=None, limit=10, status=200, per_version=False
Expand Down
31 changes: 2 additions & 29 deletions readthedocs/builds/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
from django.db import models
from django.urls import reverse
from django.utils import timezone
from django.utils.translation import gettext
from django.utils.translation import gettext_lazy as _
from django_extensions.db.models import TimeStampedModel
from polymorphic.models import PolymorphicModel
Expand Down Expand Up @@ -210,13 +209,7 @@ class Meta:
ordering = ["-verbose_name"]

def __str__(self):
return gettext(
"Version {version} of {project} ({pk})".format(
version=self.verbose_name,
project=self.project,
pk=self.pk,
),
)
return self.verbose_name

@property
def is_private(self):
Expand Down Expand Up @@ -940,17 +933,6 @@ def save(self, *args, **kwargs): # noqa
super().save(*args, **kwargs)
self._config_changed = False

def __str__(self):
return gettext(
"Build {project} for {usernames} ({pk})".format(
project=self.project,
usernames=" ".join(
self.project.users.all().values_list("username", flat=True),
),
pk=self.pk,
),
)

def get_absolute_url(self):
return reverse("builds_detail", args=[self.project.slug, self.pk])

Expand Down Expand Up @@ -1159,11 +1141,6 @@ class Meta:

objects = RelatedBuildQuerySet.as_manager()

def __str__(self):
return gettext("Build command {pk} for build {build}").format(
pk=self.pk, build=self.build
)

@property
def run_time(self):
"""Total command runtime in seconds."""
Expand Down Expand Up @@ -1347,11 +1324,7 @@ def get_edit_url(self):

def __str__(self):
class_name = self.__class__.__name__
return (
f"({self.priority}) "
f"{class_name}/{self.get_action_display()} "
f"for {self.project.slug}:{self.get_version_type_display()}"
)
return f"({self.priority}) {class_name}/{self.get_action_display()}"


class RegexAutomationRule(VersionAutomationRule):
Expand Down
7 changes: 4 additions & 3 deletions readthedocs/builds/querysets.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
EXTERNAL,
)
from readthedocs.core.permissions import AdminPermission
from readthedocs.core.querysets import NoReprQuerySet
from readthedocs.core.utils.extend import SettingsOverrideObject
from readthedocs.projects import constants
from readthedocs.projects.models import Project
Expand All @@ -23,7 +24,7 @@
__all__ = ["VersionQuerySet", "BuildQuerySet", "RelatedBuildQuerySet"]


class VersionQuerySetBase(models.QuerySet):
class VersionQuerySetBase(NoReprQuerySet, models.QuerySet):

"""Versions take into account their own privacy_level setting."""

Expand Down Expand Up @@ -145,7 +146,7 @@ class VersionQuerySet(SettingsOverrideObject):
_default_class = VersionQuerySetBase


class BuildQuerySet(models.QuerySet):
class BuildQuerySet(NoReprQuerySet, models.QuerySet):

"""
Build objects that are privacy aware.
Expand Down Expand Up @@ -269,7 +270,7 @@ def concurrent(self, project):
return (limit_reached, concurrent, max_concurrent)


class RelatedBuildQuerySet(models.QuerySet):
class RelatedBuildQuerySet(NoReprQuerySet, models.QuerySet):

"""
For models with association to a project through :py:class:`Build`.
Expand Down
4 changes: 0 additions & 4 deletions readthedocs/core/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
from django.contrib.auth.models import User
from django.db import models
from django.urls import reverse
from django.utils.translation import gettext
from django.utils.translation import gettext_lazy as _
from django_extensions.db.models import TimeStampedModel
from simple_history import register
Expand Down Expand Up @@ -46,9 +45,6 @@ class UserProfile(TimeStampedModel):
# Model history
history = ExtraHistoricalRecords()

def __str__(self):
return gettext("%(username)s's profile") % {"username": self.user.username}

def get_absolute_url(self):
return reverse(
"profiles_profile_detail",
Expand Down
15 changes: 15 additions & 0 deletions readthedocs/core/querysets.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
class NoReprQuerySet:

"""
Basic queryset to override `__repr__` function due to logging issues.
This may be a temporary solution for now and it can be improved to detect
if we are under DEBUG and/or on an interactive shell.
https://github.com/readthedocs/readthedocs.org/issues/10954
https://github.com/readthedocs/readthedocs.org/issues/10954#issuecomment-2057596044
https://github.com/readthedocs/readthedocs.org/issues/10954#issuecomment-2057951300
"""

def __repr__(self):
return self.__class__.__name__
4 changes: 1 addition & 3 deletions readthedocs/integrations/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -309,9 +309,7 @@ class Integration(TimeStampedModel):
has_sync = False

def __str__(self):
return _("{0} for {1}").format(
self.get_integration_type_display(), self.project.name
)
return self.get_integration_type_display()

def save(self, *args, **kwargs):
if not self.secret:
Expand Down
12 changes: 5 additions & 7 deletions readthedocs/invitations/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,11 @@ def invite(self, from_user, obj, to_user=None, to_email=None, request=None):
"""
if not to_user and not to_email:
raise ValueError("A user or email must be provided")
fields = dict(
content_type=ContentType.objects.get_for_model(obj),
object_id=obj.pk,
)

fields = {
"content_type": ContentType.objects.get_for_model(obj),
"object_id": obj.pk,
}
if to_user:
fields["to_user"] = to_user
else:
Expand Down Expand Up @@ -241,6 +242,3 @@ def create_audit_log(self, action, request, user=None):
user=user,
**kwargs,
)

def __str__(self):
return f"Invitation for {self.username} to join {self.object}"
3 changes: 2 additions & 1 deletion readthedocs/notifications/querysets.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@
from django.utils import timezone

from readthedocs.core.permissions import AdminPermission
from readthedocs.core.querysets import NoReprQuerySet

from .constants import CANCELLED, READ, UNREAD


class NotificationQuerySet(models.QuerySet):
class NotificationQuerySet(NoReprQuerySet, models.QuerySet):
def add(self, *args, **kwargs):
"""
Create a notification without duplicating it.
Expand Down
10 changes: 2 additions & 8 deletions readthedocs/oauth/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ class Meta:
db_table = "oauth_remoteorganization_2020"

def __str__(self):
return "Remote organization: {name}".format(name=self.slug)
return self.slug

def get_remote_organization_relation(self, user, social_account):
"""Return RemoteOrganizationRelation object for the remote organization."""
Expand Down Expand Up @@ -96,9 +96,6 @@ class Meta:
"account",
)

def __str__(self):
return f"{self.user.username} <-> {self.remote_organization.name}"


class RemoteRepository(TimeStampedModel):

Expand Down Expand Up @@ -189,7 +186,7 @@ class Meta:
db_table = "oauth_remoterepository_2020"

def __str__(self):
return "Remote repository: {}".format(self.html_url)
return self.html_url

def matches(self, user):
"""Existing projects connected to this RemoteRepository."""
Expand Down Expand Up @@ -250,6 +247,3 @@ class Meta:
"remote_repository",
"account",
)

def __str__(self):
return f"{self.user.username} <-> {self.remote_repository.full_name}"
4 changes: 3 additions & 1 deletion readthedocs/oauth/querysets.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@

from django.db import models

from readthedocs.core.querysets import NoReprQuerySet

class RelatedUserQuerySet(models.QuerySet):

class RelatedUserQuerySet(NoReprQuerySet, models.QuerySet):

"""For models with relations through :py:class:`User`."""

Expand Down
21 changes: 1 addition & 20 deletions readthedocs/organizations/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -214,12 +214,6 @@ class OrganizationOwner(models.Model):
on_delete=models.CASCADE,
)

def __str__(self):
return _("{org} owner {owner}").format(
org=self.organization.name,
owner=self.owner.username,
)


class Team(models.Model):

Expand Down Expand Up @@ -325,10 +319,7 @@ class Meta:
unique_together = ("team", "email")

def __str__(self):
return "{email} to {team}".format(
email=self.email,
team=self.team,
)
return self.email

def save(self, *args, **kwargs):
hash_ = salted_hmac(
Expand Down Expand Up @@ -399,16 +390,6 @@ class Meta:

objects = TeamMemberManager()

def __str__(self):
state = ""
if self.is_invite:
state = " (pending)"
return "{username} to {team}{state}".format(
username=self.username,
team=self.team,
state=state,
)

@property
def username(self):
"""Return member username or invite email as username."""
Expand Down
3 changes: 2 additions & 1 deletion readthedocs/organizations/querysets.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,12 @@
from django.utils import timezone
from djstripe.enums import InvoiceStatus, SubscriptionStatus

from readthedocs.core.querysets import NoReprQuerySet
from readthedocs.core.utils.extend import SettingsOverrideObject
from readthedocs.subscriptions.constants import DISABLE_AFTER_DAYS


class BaseOrganizationQuerySet(models.QuerySet):
class BaseOrganizationQuerySet(NoReprQuerySet, models.QuerySet):

"""Organizations queryset."""

Expand Down
20 changes: 3 additions & 17 deletions readthedocs/projects/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,6 @@ class ProjectRelationship(models.Model):

objects = ChildRelatedProjectQuerySet.as_manager()

def __str__(self):
return "{} -> {}".format(self.parent, self.child)

def save(self, *args, **kwargs):
if not self.alias:
self.alias = self.child.slug
Expand Down Expand Up @@ -1506,9 +1503,6 @@ def get_absolute_url(self):
filename=self.path,
)

def __str__(self):
return "{}: {}".format(self.name, self.project)


class HTMLFile(ImportedFile):

Expand Down Expand Up @@ -1696,9 +1690,6 @@ def sign_payload(self, payload):
)
return digest.hexdigest()

def __str__(self):
return f"{self.project.slug} {self.url}"


class Domain(TimeStampedModel):

Expand Down Expand Up @@ -1790,10 +1781,7 @@ class Meta:
ordering = ("-canonical", "-machine", "domain")

def __str__(self):
return "{domain} pointed at {project}".format(
domain=self.domain,
project=self.project.name,
)
return self.domain

@property
def is_valid(self):
Expand Down Expand Up @@ -1864,7 +1852,7 @@ class HTTPHeader(TimeStampedModel, models.Model):
)

def __str__(self):
return f"HttpHeader: {self.name} on {self.domain.domain}"
return self.name


class Feature(models.Model):
Expand Down Expand Up @@ -2033,9 +2021,7 @@ def add_features(sender, **kwargs):
objects = FeatureQuerySet.as_manager()

def __str__(self):
return "{} feature".format(
self.get_feature_display(),
)
return self.get_feature_display()

def get_feature_display(self):
"""
Expand Down
7 changes: 4 additions & 3 deletions readthedocs/projects/querysets.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@
from django.db.models import Count, OuterRef, Prefetch, Q, Subquery

from readthedocs.core.permissions import AdminPermission
from readthedocs.core.querysets import NoReprQuerySet
from readthedocs.core.utils.extend import SettingsOverrideObject
from readthedocs.projects import constants
from readthedocs.subscriptions.products import get_feature


class ProjectQuerySetBase(models.QuerySet):
class ProjectQuerySetBase(NoReprQuerySet, models.QuerySet):

"""Projects take into account their own privacy_level setting."""

Expand Down Expand Up @@ -169,7 +170,7 @@ class ProjectQuerySet(SettingsOverrideObject):
_default_class = ProjectQuerySetBase


class RelatedProjectQuerySet(models.QuerySet):
class RelatedProjectQuerySet(NoReprQuerySet, models.QuerySet):

"""
Useful for objects that relate to Project and its permissions.
Expand Down Expand Up @@ -220,7 +221,7 @@ class ChildRelatedProjectQuerySet(RelatedProjectQuerySet):
use_for_related_fields = True


class FeatureQuerySet(models.QuerySet):
class FeatureQuerySet(NoReprQuerySet, models.QuerySet):
use_for_related_fields = True

def for_project(self, project):
Expand Down
Loading

0 comments on commit 496f3c8

Please sign in to comment.