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

Add modeling for intersphinx data #5289

Merged
merged 34 commits into from
Feb 27, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
83c6f40
Remove old test references to intersphinx
ericholscher Oct 23, 2018
db5fbd8
Initial commit of domains app
ericholscher Oct 23, 2018
c2cd164
Merge branch 'allow-local-files-in-dev' into intersphinx-modeling
ericholscher Oct 24, 2018
41b2857
Add initial domaindata modeling and API integration
ericholscher Oct 30, 2018
806d3c2
Merge remote-tracking branch 'origin/master' into intersphinx-modeling
ericholscher Oct 30, 2018
0f82c26
Add indexing of DomainData to the builds.
ericholscher Oct 30, 2018
639b219
Merge remote-tracking branch 'origin/master' into intersphinx-modeling
ericholscher Jan 31, 2019
67d909a
Merge branch 'readd-search-signals' into intersphinx-modeling
ericholscher Jan 31, 2019
537c66a
Cleanup of some domain stuff
ericholscher Jan 31, 2019
647ae3f
Add search to Domains :tada:
ericholscher Jan 31, 2019
5b7d599
Merge branch 'readd-search-signals' into intersphinx-modeling
ericholscher Jan 31, 2019
1dfabe8
Add initial search implementation to DomainData
ericholscher Jan 31, 2019
08296ae
Move project search to a subset of site search
ericholscher Feb 1, 2019
1e087fb
Small cleanup around faceting
ericholscher Feb 1, 2019
12bdf04
Limit resuts by DomainData type
ericholscher Feb 4, 2019
8e015c9
Merge remote-tracking branch 'origin/master' into intersphinx-modeling
ericholscher Feb 13, 2019
444e739
Remove old domains app
ericholscher Feb 13, 2019
1672421
Remove search changes
ericholscher Feb 13, 2019
462545e
Missed a few
ericholscher Feb 13, 2019
f53e589
Missed a bit more
ericholscher Feb 13, 2019
98aae46
remove random bits that got merged in
ericholscher Feb 13, 2019
c934046
Undo url fix
ericholscher Feb 14, 2019
96fa45a
Fix linting issues
ericholscher Feb 14, 2019
d8d75d5
Merge remote-tracking branch 'origin/master' into intersphinx-modeling
ericholscher Feb 20, 2019
09b3c80
Fix tests and linting
ericholscher Feb 20, 2019
0da9283
Review feedback
ericholscher Feb 20, 2019
1bc31d0
Fix lint
ericholscher Feb 20, 2019
97851c4
Move sphinx to a runtime requirement.
ericholscher Feb 20, 2019
021e12d
Use timestamped model for standardized class names
ericholscher Feb 20, 2019
ab7a30c
Address review feedback
ericholscher Feb 22, 2019
fa72754
Merge remote-tracking branch 'origin/master' into intersphinx-modeling
ericholscher Feb 27, 2019
4ea9121
Fix API name
ericholscher Feb 27, 2019
ec71993
Fix test too
ericholscher Feb 27, 2019
18877c1
Update readthedocs/projects/tasks.py
stsewd Feb 27, 2019
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
19 changes: 11 additions & 8 deletions readthedocs/projects/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@
)
from readthedocs.doc_builder.loader import get_builder_class
from readthedocs.doc_builder.python_environments import Conda, Virtualenv
from readthedocs.domaindata.models import DomainData
from readthedocs.sphinx_domains.models import SphinxDomain
from readthedocs.projects.models import APIProject
from readthedocs.restapi.client import api as api_v2
from readthedocs.vcs_support import utils as vcs_support_utils
Expand Down Expand Up @@ -1160,7 +1160,8 @@ def _update_intersphinx_data(version, path, commit):
"""
object_file = os.path.join(path, 'objects.inv')

# These classes are from copied from Sphinx tests
# These classes are from copied from Sphinx
ericholscher marked this conversation as resolved.
Show resolved Hide resolved
# https://git.io/fhFbI
class MockConfig:
intersphinx_timeout = None
tls_verify = False
Expand All @@ -1173,16 +1174,18 @@ def warn(self, msg):
log.warning('Sphinx MockApp: %s', msg)

invdata = intersphinx.fetch_inventory(MockApp(), '', object_file)
for key in sorted(invdata or {}):
for key, value in sorted(invdata.items() or {}):
domain, _type = key.split(':')
for name, einfo in sorted(invdata[key].items()):
for name, einfo in sorted(value.items()):
# project, version, url, display_name
# ('Sphinx', '1.7.9', 'faq.html#epub-faq', 'Epub info')
url = einfo[2]
Copy link
Member

Choose a reason for hiding this comment

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

I'd add a small comment with previous to this line, with the expected structure of this einfo object. Like

# (key, name, url, doc type, etc)
# ('py', 'function', '/some-nice-url/python-function/', 'func', ...)

or... a link to the docs.

I found debugging this complicated later.

if '#' in url:
doc_name, anchor = url.split('#')
Copy link
Member

Choose a reason for hiding this comment

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

We could use urlparse https://docs.python.org/3/library/urllib.parse.html, but the logic here is very simple, so not really sure.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yea, feels unnecessary. It's also not a full URL, so it might be confused by it.

else:
doc_name, anchor = url, ''
display_name = einfo[3]
obj, _ = DomainData.objects.get_or_create(
obj, _ = SphinxDomain.objects.get_or_create(
project=version.project,
version=version,
domain=domain,
Expand All @@ -1195,9 +1198,9 @@ def warn(self, msg):
if obj.commit != commit:
obj.commit = commit
obj.save()
DomainData.objects.filter(project=version.project,
version=version
).exclude(commit=commit).delete()
SphinxDomain.objects.filter(project=version.project,
version=version
).exclude(commit=commit).delete()


def _manage_imported_files(version, path, commit):
Expand Down
4 changes: 2 additions & 2 deletions readthedocs/restapi/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
SocialAccountViewSet,
VersionViewSet,
)
from readthedocs.domaindata.api import DomainDataAPIView
from readthedocs.sphinx_domains.api import SphinxDomainAPIView


router = routers.DefaultRouter()
Expand All @@ -35,7 +35,7 @@
router.register(r'project', ProjectViewSet, basename='project')
router.register(r'notification', NotificationViewSet, basename='emailhook')
router.register(r'domain', DomainViewSet, basename='domain')
router.register(r'domaindata', DomainDataAPIView, base_name='domaindata')
router.register(r'sphinx_domains', SphinxDomainAPIView, base_name='sphinx_domains')
router.register(
r'remote/org',
RemoteOrganizationViewSet,
Expand Down
2 changes: 1 addition & 1 deletion readthedocs/rtd_tests/tests/test_privacy_urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ def setUp(self):
'api_webhook_gitlab': {'status_code': 405},
'api_webhook_bitbucket': {'status_code': 405},
'api_webhook_generic': {'status_code': 403},
'domaindata-detail': {'status_code': 404},
'sphinx_domains-detail': {'status_code': 404},
Copy link
Member

Choose a reason for hiding this comment

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

I think it doesn't matter too much, but to keep consistency here with the other endpoints, I'd say that it should be called sphinxdomains-detail (without the _).

Copy link
Member Author

Choose a reason for hiding this comment

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

This is autogenerated by REST framework.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed. It also shouldn't be plural. Fixed.

'remoteorganization-detail': {'status_code': 404},
'remoterepository-detail': {'status_code': 404},
'remoteaccount-detail': {'status_code': 404},
Expand Down
2 changes: 1 addition & 1 deletion readthedocs/settings/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ def INSTALLED_APPS(self): # noqa
'readthedocs.notifications',
'readthedocs.integrations',
'readthedocs.analytics',
'readthedocs.domaindata',
'readthedocs.sphinx_domains',
'readthedocs.search',


Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
"""Domain Admin classes."""
from django.contrib import admin

from .models import DomainData
from .models import SphinxDomain


class DomainDataAdmin(admin.ModelAdmin):
class SphinxDomainAdmin(admin.ModelAdmin):
list_filter = ('type', 'project')
raw_id_fields = ('project', 'version')
search_fields = ('doc_name', 'name')


admin.site.register(DomainData, DomainDataAdmin)
admin.site.register(SphinxDomain, SphinxDomainAdmin)
22 changes: 11 additions & 11 deletions readthedocs/domaindata/api.py → readthedocs/sphinx_domains/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,35 +4,35 @@

from readthedocs.restapi.views.model_views import UserSelectViewSet

from .models import DomainData
from .models import SphinxDomain


class DomainDataSerializer(serializers.ModelSerializer):
class SphinxDomainSerializer(serializers.ModelSerializer):
project = serializers.SlugRelatedField(slug_field='slug', read_only=True)
version = serializers.SlugRelatedField(slug_field='slug', read_only=True)

class Meta:
model = DomainData
model = SphinxDomain
fields = (
'project',
'version',
'name',
'display_name',
'doc_type',
'doc_url',
'role_name',
'docs_url',
)


class DomainDataAdminSerializer(DomainDataSerializer):
class SphinxDomainAdminSerializer(SphinxDomainSerializer):

class Meta(DomainDataSerializer.Meta):
class Meta(SphinxDomainSerializer.Meta):
fields = '__all__'


class DomainDataAPIView(UserSelectViewSet): # pylint: disable=too-many-ancestors
model = DomainData
serializer_class = DomainDataSerializer
admin_serializer_class = DomainDataAdminSerializer
class SphinxDomainAPIView(UserSelectViewSet): # pylint: disable=too-many-ancestors
model = SphinxDomain
serializer_class = SphinxDomainSerializer
admin_serializer_class = SphinxDomainAdminSerializer
filter_fields = (
'project__slug',
'version__slug',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
from readthedocs.projects.querysets import RelatedProjectQuerySet


class DomainData(TimeStampedModel):
class SphinxDomain(TimeStampedModel):

"""
Information from a project about it's Sphinx domains.
Expand All @@ -25,14 +25,14 @@ class DomainData(TimeStampedModel):

project = models.ForeignKey(
Project,
related_name='domain_data',
related_name='sphinx_domains',
)
version = models.ForeignKey(
Version,
verbose_name=_('Version'),
related_name='domain_data',
related_name='sphinx_domains',
)
commit = models.CharField(_('Commit'), max_length=255)
commit = models.CharField(_('Commit'), max_length=255, null=True)

domain = models.CharField(
_('Domain'),
Expand Down Expand Up @@ -62,16 +62,17 @@ class DomainData(TimeStampedModel):

def __str__(self):
return f'''
DomainData [{self.project.slug}:{self.version.slug}]
[{self.domain}:{self.type}] {self.name} -> {self.doc_name}#{self.anchor}
SphinxDomain [{self.project.slug}:{self.version.slug}]
[{self.domain}:{self.type}] {self.name} ->
{self.doc_name}#{self.anchor}
'''

@property
def doc_type(self):
def role_name(self):
return f'{self.domain}:{self.type}'

@property
def doc_url(self):
def docs_url(self):
path = self.doc_name
if self.anchor:
path += f'#{self.anchor}'
Expand Down