Skip to content
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
40 changes: 40 additions & 0 deletions readthedocs/api/v2/utils.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"""Utility functions that are used by both views and celery tasks."""

import itertools
import json
import re

import structlog
Expand All @@ -18,6 +19,7 @@
from readthedocs.builds.models import RegexAutomationRule
from readthedocs.builds.models import Version
from readthedocs.core.utils.db import delete_in_batches
from readthedocs.storage import build_commands_storage


log = structlog.get_logger(__name__)
Expand Down Expand Up @@ -285,6 +287,44 @@ def normalize_build_command(command, project_slug, version_slug):
return command


def get_build_commands_from_storage(build):
"""
Return build commands from storage for ``cold_storage`` builds.

Returns ``None`` when commands can't be loaded from storage and callers
should keep using serializer output.
"""
if not settings.RTD_SAVE_BUILD_COMMANDS_TO_STORAGE:
return None

if not build.cold_storage:
return None

storage_path = "{date}/{id}.json".format(
date=str(build.date.date()),
id=build.id,
)
if not build_commands_storage.exists(storage_path):
return None

try:
json_resp = build_commands_storage.open(storage_path).read()
commands = json.loads(json_resp)
for buildcommand in commands:
buildcommand["command"] = normalize_build_command(
buildcommand["command"],
build.project.slug,
build.get_version_slug(),
)
return commands
except Exception:
log.exception(
"Failed to read build data from storage.",
path=storage_path,
)
return None


class RemoteOrganizationPagination(PageNumberPagination):
page_size = 25

Expand Down
34 changes: 5 additions & 29 deletions readthedocs/api/v2/views/model_views.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
"""Endpoints for listing Projects, Versions, Builds, etc."""

import json
from dataclasses import asdict

import structlog
Expand Down Expand Up @@ -28,7 +27,7 @@
from readthedocs.api.v2.permissions import HasBuildAPIKey
from readthedocs.api.v2.permissions import IsOwner
from readthedocs.api.v2.permissions import ReadOnlyPermission
from readthedocs.api.v2.utils import normalize_build_command
from readthedocs.api.v2.utils import get_build_commands_from_storage
from readthedocs.aws.security_token_service import AWSTemporaryCredentialsError
from readthedocs.aws.security_token_service import get_s3_build_media_scoped_credentials
from readthedocs.aws.security_token_service import get_s3_build_tools_scoped_credentials
Expand All @@ -43,7 +42,6 @@
from readthedocs.oauth.services import registry
from readthedocs.projects.models import Domain
from readthedocs.projects.models import Project
from readthedocs.storage import build_commands_storage

from ..serializers import BuildAdminReadOnlySerializer
from ..serializers import BuildAdminSerializer
Expand Down Expand Up @@ -336,35 +334,13 @@ def retrieve(self, *args, **kwargs):
This uses files from storage to get the JSON,
and replaces the ``commands`` part of the response data.
"""
if not settings.RTD_SAVE_BUILD_COMMANDS_TO_STORAGE:
return super().retrieve(*args, **kwargs)

instance = self.get_object()
serializer = self.get_serializer(instance)
data = serializer.data
if instance.cold_storage:
storage_path = "{date}/{id}.json".format(
date=str(instance.date.date()),
id=instance.id,
)
if build_commands_storage.exists(storage_path):
try:
json_resp = build_commands_storage.open(storage_path).read()
data["commands"] = json.loads(json_resp)

# Normalize commands in the same way than when returning
# them using the serializer
for buildcommand in data["commands"]:
buildcommand["command"] = normalize_build_command(
buildcommand["command"],
instance.project.slug,
instance.get_version_slug(),
)
except Exception:
log.exception(
"Failed to read build data from storage.",
path=storage_path,
)
commands = get_build_commands_from_storage(instance)
if commands is not None:
data["commands"] = commands

return Response(data)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems we are changing the logic here when not settings.RTD_SAVE_BUILD_COMMANDS_TO_STORAGE.
The old code runs return super().retrieve(*args, **kwargs) and now we will be running return Response(serializer.data) instead.

Is that the same?


@decorators.action(
Expand Down
48 changes: 48 additions & 0 deletions readthedocs/api/v3/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,11 @@
from taggit.serializers import TaggitSerializer
from taggit.serializers import TagListSerializerField

from readthedocs.api.v2.utils import normalize_build_command
from readthedocs.builds.constants import LATEST
from readthedocs.builds.constants import STABLE
from readthedocs.builds.models import Build
from readthedocs.builds.models import BuildCommandResult
from readthedocs.builds.models import Version
from readthedocs.core.permissions import AdminPermission
from readthedocs.core.resolver import Resolver
Expand Down Expand Up @@ -143,6 +145,32 @@ def get_version(self, obj):
return None


class BuildCommandSerializer(serializers.ModelSerializer):
run_time = serializers.ReadOnlyField()
command = serializers.SerializerMethodField()

class Meta:
model = BuildCommandResult
fields = [
"id",
"build",
"command",
"description",
"output",
"exit_code",
"start_time",
"end_time",
"run_time",
Comment on lines +155 to +163
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can probably remove these fields since we are not using them.

Suggested change
"id",
"build",
"command",
"description",
"output",
"exit_code",
"start_time",
"end_time",
"run_time",
"id",
"command",
"output",
"exit_code",
"start_time",
"end_time",
"run_time",

]

def get_command(self, obj):
return normalize_build_command(
obj.command,
obj.build.project.slug,
obj.build.get_version_slug(),
)


class BuildConfigSerializer(FlexFieldsSerializerMixin, serializers.Serializer):
"""
Render ``Build.config`` property without modifying it.
Expand Down Expand Up @@ -177,6 +205,10 @@ class BuildSerializer(FlexFieldsModelSerializer):
state = BuildStateSerializer(source="*")
_links = BuildLinksSerializer(source="*")
urls = BuildURLsSerializer(source="*")
builder = serializers.CharField(read_only=True)
docs_url = serializers.SerializerMethodField()
commit_url = serializers.SerializerMethodField()
Comment on lines +209 to +210
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These attributes should be inside the urls field. We have a specific serializer to add more of these: BuildURLsSerializer

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, we should use urls.documenation instead of urls.docs to keep consistency with the VersionURLsSerializer

commands = BuildCommandSerializer(many=True, read_only=True)

class Meta:
model = Build
Expand All @@ -191,6 +223,10 @@ class Meta:
"success",
"error",
"commit",
"builder",
"docs_url",
"commit_url",
"commands",
"_links",
"urls",
]
Expand All @@ -212,6 +248,18 @@ def get_success(self, obj):

return None

def get_docs_url(self, obj):
if obj.version:
return obj.version.get_absolute_url()

return None

def get_commit_url(self, obj):
if obj.commit:
return obj.get_commit_url()

return None


class NotificationMessageSerializer(serializers.Serializer):
id = serializers.SlugField()
Expand Down
24 changes: 19 additions & 5 deletions readthedocs/api/v3/tests/mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,10 @@
from rest_framework.authtoken.models import Token
from rest_framework.test import APIClient

from readthedocs.builds.constants import LATEST, TAG
from readthedocs.builds.models import Build, Version
from readthedocs.builds.constants import LATEST
from readthedocs.builds.constants import TAG
from readthedocs.builds.models import Build
from readthedocs.builds.models import Version
from readthedocs.core.notifications import MESSAGE_EMAIL_VALIDATION_PENDING
from readthedocs.doc_builder.exceptions import BuildCancelled
from readthedocs.notifications.models import Notification
Expand Down Expand Up @@ -104,13 +106,14 @@ def setUp(self):
state="finished",
error="",
success=True,
_config={"property": "test value"},
version=self.version,
project=self.project,
builder="builder01",
commit="a1b2c3",
length=60,
)
self.build.config = {"property": "test value"}
self.build.save()
Comment on lines +115 to +116
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we moving this outside the fixture.get? We should be able to put it there to avoid an extra DB call.


self.other = fixture.get(User, projects=[])
self.others_token = fixture.get(Token, key="other", user=self.other)
Expand All @@ -134,13 +137,14 @@ def setUp(self):
state="finished",
error="",
success=True,
_config={"property": "test value"},
version=self.others_version,
project=self.others_project,
builder="builder01",
commit="a1b2c3",
length=60,
)
self.others_build.config = {"property": "test value"}
self.others_build.save()

# Make all non-html true so responses are complete
self.project.versions.update(
Expand Down Expand Up @@ -245,7 +249,11 @@ def _get_response_dict(self, view_name, filepath=None):
filename = Path(filepath).absolute().parent / "responses" / f"{view_name}.json"
return json.load(open(filename))

def assertDictEqual(self, d1, d2):
# Keep unittest.TestCase signature compatibility.
# assertEqual() dispatches dict comparisons to assertDictEqual and passes
# `msg` as a keyword argument:
# https://docs.python.org/3/library/unittest.html#unittest.TestCase.assertEqual
def assertDictEqual(self, d1, d2, msg=None):
"""
Show the differences between the dicts in a human readable way.

Expand All @@ -258,4 +266,10 @@ def assertDictEqual(self, d1, d2):
message = datadiff.diff(d1, d2)
except ImportError:
pass

if msg and message:
message = f"{message}\n{msg}"
elif msg:
message = msg

return super().assertDictEqual(d1, d2, message)
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@
"error": "",
"finished": "2019-04-29T10:01:00Z",
"id": 1,
"builder": "builder01",
"docs_url": "http://project.readthedocs.io/en/v1.0/",
"commit_url": "https://github.com/rtfd/project/commit/a1b2c3",
"commands": [],
"_links": {
"_self": "https://readthedocs.org/api/v3/projects/project/builds/1/",
"project": "https://readthedocs.org/api/v3/projects/project/",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@
"error": "",
"finished": "2019-04-29T10:01:00Z",
"id": 1,
"builder": "builder01",
"docs_url": "http://project.readthedocs.io/en/v1.0/",
"commit_url": "https://github.com/rtfd/project/commit/a1b2c3",
"commands": [],
"_links": {
"_self": "https://readthedocs.org/api/v3/projects/project/builds/1/",
"project": "https://readthedocs.org/api/v3/projects/project/",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@
"error": "",
"finished": null,
"id": 3,
"builder": null,
"docs_url": "http://project.readthedocs.io/en/v1.0/",
"commit_url": null,
"commands": [],
"_links": {
"_self": "https://readthedocs.org/api/v3/projects/project/builds/3/",
"notifications": "https://readthedocs.org/api/v3/projects/project/builds/3/notifications/",
Expand Down
Loading