Skip to content

Refactor tags/branches data to use dataclass instead of dicts (fixes #7798) #12364

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
14 changes: 12 additions & 2 deletions readthedocs/api/v2/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from readthedocs.builds.constants import STABLE
from readthedocs.builds.constants import STABLE_VERBOSE_NAME
from readthedocs.builds.constants import TAG
from readthedocs.builds.datatypes import VersionData
from readthedocs.builds.models import RegexAutomationRule
from readthedocs.builds.models import Version

Expand Down Expand Up @@ -168,10 +169,13 @@ def _set_or_create_version(project, slug, version_id, verbose_name, type_):


def _get_deleted_versions_qs(project, tags_data, branches_data):
# Convert to VersionData if not already
tags_data = [VersionData(**v) if not isinstance(v, VersionData) else v for v in tags_data]
branches_data = [VersionData(**v) if not isinstance(v, VersionData) else v for v in branches_data]
# We use verbose_name for tags
# because several tags can point to the same identifier.
versions_tags = [version["verbose_name"] for version in tags_data]
versions_branches = [version["identifier"] for version in branches_data]
versions_tags = [version.verbose_name for version in tags_data]
versions_branches = [version.identifier for version in branches_data]

to_delete_qs = (
project.versions(manager=INTERNAL)
Expand All @@ -196,6 +200,9 @@ def delete_versions_from_db(project, tags_data, branches_data):

:returns: The slug of the deleted versions from the database.
"""
# Convert to VersionData if not already
tags_data = [VersionData(**v) if not isinstance(v, VersionData) else v for v in tags_data]
branches_data = [VersionData(**v) if not isinstance(v, VersionData) else v for v in branches_data]
to_delete_qs = _get_deleted_versions_qs(
project=project,
tags_data=tags_data,
Expand All @@ -212,6 +219,9 @@ def delete_versions_from_db(project, tags_data, branches_data):

def get_deleted_active_versions(project, tags_data, branches_data):
"""Return the slug of active versions that were deleted from the repository."""
# Convert to VersionData if not already
tags_data = [VersionData(**v) if not isinstance(v, VersionData) else v for v in tags_data]
branches_data = [VersionData(**v) if not isinstance(v, VersionData) else v for v in branches_data]
to_delete_qs = _get_deleted_versions_qs(
project=project,
tags_data=tags_data,
Expand Down
6 changes: 6 additions & 0 deletions readthedocs/builds/datatypes.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
from dataclasses import dataclass

@dataclass
class VersionData:
identifier: str
verbose_name: str
16 changes: 10 additions & 6 deletions readthedocs/builds/tasks.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import json
from io import BytesIO
import dataclasses

import requests
import structlog
Expand All @@ -25,6 +26,7 @@
from readthedocs.builds.constants import LOCK_EXPIRE
from readthedocs.builds.constants import MAX_BUILD_COMMAND_SIZE
from readthedocs.builds.constants import TAG
from readthedocs.builds.datatypes import VersionData
from readthedocs.builds.models import Build
from readthedocs.builds.models import Version
from readthedocs.builds.reporting import get_build_overview
Expand Down Expand Up @@ -294,6 +296,8 @@ def sync_versions_task(project_pk, tags_data, branches_data, **kwargs):
].
:returns: `True` or `False` if the task succeeded.
"""
tags_data = [VersionData(**d) for d in tags_data]
branches_data = [VersionData(**d) for d in branches_data]
project = Project.objects.get(pk=project_pk)

# If the currently highest non-prerelease version is active, then make
Expand All @@ -309,27 +313,27 @@ def sync_versions_task(project_pk, tags_data, branches_data, **kwargs):
added_versions = set()
result = sync_versions_to_db(
project=project,
versions=tags_data,
versions=[dataclasses.asdict(v) for v in tags_data],
type=TAG,
)
added_versions.update(result)

result = sync_versions_to_db(
project=project,
versions=branches_data,
versions=[dataclasses.asdict(v) for v in branches_data],
type=BRANCH,
)
added_versions.update(result)

delete_versions_from_db(
project=project,
tags_data=tags_data,
branches_data=branches_data,
tags_data=[dataclasses.asdict(v) for v in tags_data],
branches_data=[dataclasses.asdict(v) for v in branches_data],
)
deleted_active_versions = get_deleted_active_versions(
project=project,
tags_data=tags_data,
branches_data=branches_data,
tags_data=[dataclasses.asdict(v) for v in tags_data],
branches_data=[dataclasses.asdict(v) for v in branches_data],
)
except Exception:
log.exception("Sync Versions Error")
Expand Down
18 changes: 7 additions & 11 deletions readthedocs/projects/tasks/mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
from readthedocs.builds.constants import LATEST_VERBOSE_NAME
from readthedocs.builds.constants import STABLE_VERBOSE_NAME
from readthedocs.builds.models import APIVersion
from readthedocs.builds.datatypes import VersionData
import dataclasses

from ..exceptions import RepositoryError
from ..models import Feature
Expand Down Expand Up @@ -50,18 +52,12 @@ def sync_versions(self, vcs_repository):
)

tags_data = [
{
"identifier": v.identifier,
"verbose_name": v.verbose_name,
}
VersionData(identifier=v.identifier, verbose_name=v.verbose_name)
for v in tags
]

branches_data = [
{
"identifier": v.identifier,
"verbose_name": v.verbose_name,
}
VersionData(identifier=v.identifier, verbose_name=v.verbose_name)
for v in branches
]

Expand All @@ -74,8 +70,8 @@ def sync_versions(self, vcs_repository):

build_tasks.sync_versions_task.delay(
project_pk=self.data.project.pk,
tags_data=tags_data,
branches_data=branches_data,
tags_data=[dataclasses.asdict(v) for v in tags_data],
branches_data=[dataclasses.asdict(v) for v in branches_data],
)

def validate_duplicate_reserved_versions(self, tags_data, branches_data):
Expand All @@ -88,7 +84,7 @@ def validate_duplicate_reserved_versions(self, tags_data, branches_data):

:param data: Dict containing the versions from tags and branches
"""
version_names = [version["verbose_name"] for version in tags_data + branches_data]
version_names = [version.verbose_name for version in tags_data + branches_data]
counter = Counter(version_names)
for reserved_name in [STABLE_VERBOSE_NAME, LATEST_VERBOSE_NAME]:
if counter[reserved_name] > 1:
Expand Down