From 59cccbff946e4744449d7353f7444a80816d6abc Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Thu, 5 Oct 2017 16:34:55 -0600 Subject: [PATCH 01/15] Add feature flipping models to Projects, plus example feature flip Also, use a new pattern for API instance objects --- .../doc_builder/python_environments.py | 11 ++- readthedocs/projects/admin.py | 11 ++- readthedocs/projects/forms.py | 22 +++++- .../projects/migrations/0019_add-features.py | 28 +++++++ readthedocs/projects/models.py | 78 +++++++++++++++++++ readthedocs/projects/utils.py | 5 +- readthedocs/restapi/serializers.py | 17 +++- 7 files changed, 162 insertions(+), 10 deletions(-) create mode 100644 readthedocs/projects/migrations/0019_add-features.py diff --git a/readthedocs/doc_builder/python_environments.py b/readthedocs/doc_builder/python_environments.py index f5c55f5f44f..707df75ca02 100644 --- a/readthedocs/doc_builder/python_environments.py +++ b/readthedocs/doc_builder/python_environments.py @@ -11,6 +11,7 @@ from readthedocs.doc_builder.config import ConfigWrapper from readthedocs.doc_builder.loader import get_builder_class from readthedocs.projects.constants import LOG_TEMPLATE +from readthedocs.projects.models import Feature log = logging.getLogger(__name__) @@ -128,8 +129,14 @@ def install_core_requirements(self): if self.project.documentation_type == 'mkdocs': requirements.append('mkdocs==0.15.0') else: - requirements.extend(['sphinx==1.5.3', 'sphinx-rtd-theme<0.3', - 'readthedocs-sphinx-ext<0.6']) + if self.project.has_feature(Feature.USE_SPHINX_LATEST): + requirements.append('sphinx<1.7') + else: + requirements.append('sphinx==1.5.6') + requirements.extend([ + 'sphinx-rtd-theme<0.3', + 'readthedocs-sphinx-ext<0.6' + ]) cmd = [ 'python', diff --git a/readthedocs/projects/admin.py b/readthedocs/projects/admin.py index c881189e63e..244173b1d4e 100644 --- a/readthedocs/projects/admin.py +++ b/readthedocs/projects/admin.py @@ -13,9 +13,10 @@ from readthedocs.redirects.models import Redirect from readthedocs.notifications.views import SendNotificationView -from .notifications import ResourceUsageNotification -from .models import (Project, ImportedFile, +from .forms import FeatureForm +from .models import (Project, ImportedFile, Feature, ProjectRelationship, EmailHook, WebHook, Domain) +from .notifications import ResourceUsageNotification from .tasks import remove_dir @@ -180,8 +181,14 @@ class DomainAdmin(admin.ModelAdmin): model = Domain +class FeatureAdmin(admin.ModelAdmin): + model = Feature + form = FeatureForm + + admin.site.register(Project, ProjectAdmin) admin.site.register(ImportedFile, ImportedFileAdmin) admin.site.register(Domain, DomainAdmin) +admin.site.register(Feature, FeatureAdmin) admin.site.register(EmailHook) admin.site.register(WebHook) diff --git a/readthedocs/projects/forms.py b/readthedocs/projects/forms.py index b3ac583fd9d..a5275705701 100644 --- a/readthedocs/projects/forms.py +++ b/readthedocs/projects/forms.py @@ -22,7 +22,7 @@ from readthedocs.projects import constants from readthedocs.projects.exceptions import ProjectSpamError from readthedocs.projects.models import ( - Project, ProjectRelationship, EmailHook, WebHook, Domain) + Project, ProjectRelationship, EmailHook, WebHook, Domain, Feature) from readthedocs.redirects.models import Redirect @@ -595,3 +595,23 @@ class Meta(object): def __init__(self, *args, **kwargs): self.project = kwargs.pop('project', None) super(ProjectAdvertisingForm, self).__init__(*args, **kwargs) + + +class FeatureForm(forms.ModelForm): + + """Project feature form for dynamic admin choices + + This form converts the CharField into a ChoiceField on display. The + underlying driver won't attempt to do validation on the choices, and so we + can dynamically populate this list. + """ + + feature = forms.ChoiceField() + + class Meta(object): + model = Feature + fields = ['project', 'feature'] + + def __init__(self, *args, **kwargs): + super(FeatureForm, self).__init__(*args, **kwargs) + self.fields['feature'].choices = Feature.FEATURES diff --git a/readthedocs/projects/migrations/0019_add-features.py b/readthedocs/projects/migrations/0019_add-features.py new file mode 100644 index 00000000000..7d15b159fe1 --- /dev/null +++ b/readthedocs/projects/migrations/0019_add-features.py @@ -0,0 +1,28 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.9.12 on 2017-10-26 14:40 +from __future__ import unicode_literals + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('projects', '0018_fix-translation-model'), + ] + + operations = [ + migrations.CreateModel( + name='Feature', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('feature', models.CharField(max_length=32, verbose_name='Project feature')), + ], + ), + migrations.AddField( + model_name='feature', + name='project', + field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='features', to='projects.Project'), + ), + ] diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index 9d898f7ddc9..de250b70107 100644 --- a/readthedocs/projects/models.py +++ b/readthedocs/projects/models.py @@ -821,6 +821,35 @@ def add_comment(self, version_slug, page, content_hash, commit, user, text): hash=content_hash, commit=commit) return node.comments.create(user=user, text=text) + def has_feature(self, feature): + """Does project have existing feature flag""" + return self.features.filter(feature=feature).exists() + + +class APIProject(Project): + + """Project object from API data deserialization + + This is to preserve the Project model methods for use in builder instances. + Properties can be read only here, as the builders don't need write access to + a number of attributes. + """ + + features = [] + + class Meta: + proxy = True + + def __init__(self, *args, **kwargs): + self.features = kwargs.pop('features', []) + super(APIProject, self).__init__(*args, **kwargs) + + def save(self, *args, **kwargs): + return 0 + + def has_feature(self, feature): + return feature in self.features + @python_2_unicode_compatible class ImportedFile(models.Model): @@ -921,3 +950,52 @@ def delete(self, *args, **kwargs): # pylint: disable=arguments-differ from readthedocs.projects import tasks broadcast(type='app', task=tasks.symlink_domain, args=[self.project.pk, self.pk, True]) super(Domain, self).delete(*args, **kwargs) + + +@python_2_unicode_compatible +class Feature(models.Model): + + """Project feature flags + + Features should generally be added here as choices, however features may + also be added dynamically from a signal in other packages. See the + FeatureForm implementation for this. + + Features can be added by external packages with the use of signals:: + + @receiver(pre_init, sender=Feature) + def add_features(sender, **kwargs): + sender.FEATURES += (('blah', 'BLAH'),) + + The FeatureForm will grab the updated list on instantiation. + """ + + # Feature constants - this is not a exhaustive list of features, features + # may be added by other packages + USE_SPHINX_LATEST = 'use_sphinx_latest' + + FEATURES = ( + (USE_SPHINX_LATEST, _('Use latest version of Sphinx')), + ) + + project = models.ForeignKey( + Project, + related_name='features', + ) + # Feature is not implemented as a ChoiceField, as we don't want validation + # at the database level on this field. Arbitrary values are allowed here. + feature = models.CharField( + _('Project feature'), + max_length=32, + ) + + def __str__(self): + return "{0} feature for {1}".format(self.get_feature_display(), self.project) + + def get_feature_display(self): + """Implement display name field for fake ChoiceField + + Because choices are implemented in FeatureForm, we need to reimplement + this here. + """ + return dict(self.FEATURES).get(self.feature, self.feature) diff --git a/readthedocs/projects/utils.py b/readthedocs/projects/utils.py index df84d5c7cab..0e1e1b891d8 100644 --- a/readthedocs/projects/utils.py +++ b/readthedocs/projects/utils.py @@ -192,11 +192,10 @@ def make_api_version(version_data): def make_api_project(project_data): """Make mock Project instance from API return""" - from readthedocs.projects.models import Project + from readthedocs.projects.models import APIProject for key in ['users', 'resource_uri', 'absolute_url', 'downloads', 'main_language_project', 'related_projects']: if key in project_data: del project_data[key] - project = Project(**project_data) - project.save = _new_save + project = APIProject(**project_data) return project diff --git a/readthedocs/restapi/serializers.py b/readthedocs/restapi/serializers.py index 1c43de42904..481e04c20dc 100644 --- a/readthedocs/restapi/serializers.py +++ b/readthedocs/restapi/serializers.py @@ -1,13 +1,13 @@ """Defines serializers for each of our models.""" from __future__ import absolute_import -from builtins import object +from builtins import object from rest_framework import serializers from readthedocs.builds.models import Build, BuildCommandResult, Version -from readthedocs.projects.models import Project, Domain from readthedocs.oauth.models import RemoteOrganization, RemoteRepository +from readthedocs.projects.models import Project, Domain class ProjectSerializer(serializers.ModelSerializer): @@ -28,6 +28,18 @@ class Meta(object): class ProjectAdminSerializer(ProjectSerializer): + """Project serializer for admin only access + + Includes special internal fields that don't need to be exposed through the + general API, mostly for fields used in the build process + """ + + features = serializers.SlugRelatedField( + many=True, + read_only=True, + slug_field='feature', + ) + class Meta(ProjectSerializer.Meta): fields = ProjectSerializer.Meta.fields + ( 'enable_epub_build', @@ -44,6 +56,7 @@ class Meta(ProjectSerializer.Meta): 'skip', 'requirements_file', 'python_interpreter', + 'features', ) From d251bbd2ba72f52986e89cf6d290ffa40a54b4bc Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Thu, 26 Oct 2017 23:14:41 -0600 Subject: [PATCH 02/15] Refactoring the make_api_* calls into the API models --- readthedocs/builds/models.py | 36 ++++++++++++++- .../core/management/commands/update_api.py | 6 ++- readthedocs/projects/models.py | 38 ++++++++++------ readthedocs/projects/tasks.py | 10 ++--- readthedocs/projects/utils.py | 45 +++---------------- 5 files changed, 76 insertions(+), 59 deletions(-) diff --git a/readthedocs/builds/models.py b/readthedocs/builds/models.py index e186db2d2c1..0c738f7992d 100644 --- a/readthedocs/builds/models.py +++ b/readthedocs/builds/models.py @@ -26,7 +26,7 @@ from readthedocs.projects.constants import (PRIVACY_CHOICES, GITHUB_URL, GITHUB_REGEXS, BITBUCKET_URL, BITBUCKET_REGEXS, PRIVATE) -from readthedocs.projects.models import Project +from readthedocs.projects.models import Project, APIProject DEFAULT_VERSION_PRIVACY_LEVEL = getattr(settings, 'DEFAULT_VERSION_PRIVACY_LEVEL', 'public') @@ -301,6 +301,40 @@ def get_bitbucket_url(self, docroot, filename, source_suffix='.rst'): ) +class APIVersion(Version): + + """Version proxy model for API data deserialization + + This replaces the pattern where API data was deserialized into a mocked + :py:cls:`Version` object. This pattern was confusing, as it was not explicit + as to what form of object you were working with -- API backed or database + backed. + + This model preserves the Version model methods, allowing for overrides on + model field differences. This model pattern will generally only be used on + builder instances, where we are interacting solely with API data. + """ + + project = None + + class Meta: + proxy = True + + def __init__(self, *args, **kwargs): + self.project = APIProject(**kwargs.pop('project', {})) + # This was replicated from readthedocs.projects.utils.make_api_project. + # I'm not certain why these need to be deleted + for key in ['resource_uri', 'absolute_url', 'downloads']: + try: + del kwargs[key] + except KeyError: + pass + super(APIVersion, self).__init__(*args, **kwargs) + + def save(self, *args, **kwargs): + return 0 + + @python_2_unicode_compatible class VersionAlias(models.Model): diff --git a/readthedocs/core/management/commands/update_api.py b/readthedocs/core/management/commands/update_api.py index 09dbbf95e2c..27d176f5ebb 100644 --- a/readthedocs/core/management/commands/update_api.py +++ b/readthedocs/core/management/commands/update_api.py @@ -10,8 +10,10 @@ import logging from django.core.management.base import BaseCommand -from readthedocs.projects import tasks + from readthedocs.api.client import api +from readthedocs.projects import tasks +from readthedocs.projects.models import APIProject log = logging.getLogger(__name__) @@ -29,6 +31,6 @@ def handle(self, *args, **options): docker = options.get('docker', False) for slug in options['projects']: project_data = api.project(slug).get() - p = tasks.make_api_project(project_data) + p = APIProject(**project_data) log.info("Building %s", p) tasks.update_docs.run(pk=p.pk, docker=docker) diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index de250b70107..90aac9e80fa 100644 --- a/readthedocs/projects/models.py +++ b/readthedocs/projects/models.py @@ -29,7 +29,6 @@ ChildRelatedProjectQuerySet ) from readthedocs.projects.templatetags.projects_tags import sort_version_aware -from readthedocs.projects.utils import make_api_version from readthedocs.projects.version_handling import determine_stable_version, version_windows from readthedocs.restapi.client import api from readthedocs.vcs_support.backends import backend_cls @@ -639,9 +638,10 @@ def get_latest_build(self, finished=True): return self.builds.filter(**kwargs).first() def api_versions(self): + from readthedocs.builds.models import APIVersion ret = [] for version_data in api.project(self.pk).active_versions.get()['versions']: - version = make_api_version(version_data) + version = APIVersion(**version_data) ret.append(version) return sort_version_aware(ret) @@ -828,11 +828,16 @@ def has_feature(self, feature): class APIProject(Project): - """Project object from API data deserialization + """Project proxy model for API data deserialization - This is to preserve the Project model methods for use in builder instances. - Properties can be read only here, as the builders don't need write access to - a number of attributes. + This replaces the pattern where API data was deserialized into a mocked + :py:cls:`Project` object. This pattern was confusing, as it was not explicit + as to what form of object you were working with -- API backed or database + backed. + + This model preserves the Project model methods, allowing for overrides on + model field differences. This model pattern will generally only be used on + builder instances, where we are interacting solely with API data. """ features = [] @@ -842,6 +847,12 @@ class Meta: def __init__(self, *args, **kwargs): self.features = kwargs.pop('features', []) + for key in ['users', 'resource_uri', 'absolute_url', 'downloads', + 'main_language_project', 'related_projects']: + try: + del kwargs[key] + except KeyError: + pass super(APIProject, self).__init__(*args, **kwargs) def save(self, *args, **kwargs): @@ -958,10 +969,8 @@ class Feature(models.Model): """Project feature flags Features should generally be added here as choices, however features may - also be added dynamically from a signal in other packages. See the - FeatureForm implementation for this. - - Features can be added by external packages with the use of signals:: + also be added dynamically from a signal in other packages. Features can be + added by external packages with the use of signals:: @receiver(pre_init, sender=Feature) def add_features(sender, **kwargs): @@ -990,12 +999,15 @@ def add_features(sender, **kwargs): ) def __str__(self): - return "{0} feature for {1}".format(self.get_feature_display(), self.project) + return "{0} feature for {1}".format( + self.get_feature_display(), + self.project, + ) def get_feature_display(self): """Implement display name field for fake ChoiceField - Because choices are implemented in FeatureForm, we need to reimplement - this here. + Because the field is not a ChoiceField here, we need to manually + implement this behavior. """ return dict(self.FEATURES).get(self.feature, self.feature) diff --git a/readthedocs/projects/tasks.py b/readthedocs/projects/tasks.py index 136af14b8e5..7d50ea0881c 100644 --- a/readthedocs/projects/tasks.py +++ b/readthedocs/projects/tasks.py @@ -29,14 +29,13 @@ from .exceptions import ProjectImportError from .models import ImportedFile, Project, Domain from .signals import before_vcs, after_vcs, before_build, after_build -from .utils import make_api_version, make_api_project from readthedocs.api.client import api as api_v1 from readthedocs.builds.constants import (LATEST, BUILD_STATE_CLONING, BUILD_STATE_INSTALLING, BUILD_STATE_BUILDING, BUILD_STATE_FINISHED) -from readthedocs.builds.models import Build, Version +from readthedocs.builds.models import Build, Version, APIVersion from readthedocs.builds.signals import build_complete from readthedocs.builds.syncers import Syncer from readthedocs.cdn.purge import purge @@ -49,6 +48,7 @@ from readthedocs.doc_builder.exceptions import BuildEnvironmentError from readthedocs.doc_builder.loader import get_builder_class from readthedocs.doc_builder.python_environments import Virtualenv, Conda +from readthedocs.projects.models import APIProject from readthedocs.restapi.client import api as api_v2 from readthedocs.restapi.utils import index_search_request from readthedocs.search.parse_json import process_all_json_files @@ -235,7 +235,7 @@ def run_build(self, docker=False, record=True): def get_project(project_pk): """Get project from API""" project_data = api_v2.project(project_pk).get() - project = make_api_project(project_data) + project = APIProject(**project_data) return project @staticmethod @@ -247,7 +247,7 @@ def get_version(project, version_pk): version_data = (api_v2 .version(project.slug) .get(slug=LATEST)['objects'][0]) - return make_api_version(version_data) + return APIVersion(**version_data) @staticmethod def get_build(build_pk): @@ -510,7 +510,7 @@ def update_imported_docs(version_pk): :param version_pk: Version id to update """ version_data = api_v2.version(version_pk).get() - version = make_api_version(version_data) + version = APIVersion(**version_data) project = version.project ret_dict = {} diff --git a/readthedocs/projects/utils.py b/readthedocs/projects/utils.py index 0e1e1b891d8..86e2040d03c 100644 --- a/readthedocs/projects/utils.py +++ b/readthedocs/projects/utils.py @@ -1,30 +1,31 @@ """Utility functions used by projects""" from __future__ import absolute_import -from builtins import object + import fnmatch +import logging import os import subprocess import traceback -import logging -from httplib2 import Http import redis import six +from builtins import object from django.conf import settings from django.core.cache import cache +from httplib2 import Http log = logging.getLogger(__name__) +# TODO make this a classmethod of Version def version_from_slug(slug, version): - from readthedocs.projects import tasks - from readthedocs.builds.models import Version + from readthedocs.builds.models import Version, APIVersion from readthedocs.restapi.client import api if getattr(settings, 'DONT_HIT_DB', True): version_data = api.version().get(project=slug, slug=version)['results'][0] - v = tasks.make_api_version(version_data) + v = APIVersion(**version_data) else: v = Version.objects.get(project__slug=slug, slug=version) return v @@ -167,35 +168,3 @@ class DictObj(object): def __getattr__(self, attr): return self.__dict__.get(attr) - - -# Prevent saving the temporary Project instance -def _new_save(*dummy_args, **dummy_kwargs): - log.warning("Called save on a non-real object.") - return 0 - - -def make_api_version(version_data): - """Make mock Version instance from API return""" - from readthedocs.builds.models import Version - for key in ['resource_uri', 'absolute_url', 'downloads']: - if key in version_data: - del version_data[key] - project_data = version_data['project'] - project = make_api_project(project_data) - version_data['project'] = project - ver = Version(**version_data) - ver.save = _new_save - - return ver - - -def make_api_project(project_data): - """Make mock Project instance from API return""" - from readthedocs.projects.models import APIProject - for key in ['users', 'resource_uri', 'absolute_url', 'downloads', - 'main_language_project', 'related_projects']: - if key in project_data: - del project_data[key] - project = APIProject(**project_data) - return project From 21bab419a0c7e49de1634922b82c216fb969badc Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Thu, 26 Oct 2017 23:18:16 -0600 Subject: [PATCH 03/15] Add migrations for proxy models --- .../0004_add-apiversion-proxy-model.py | 24 +++++++++++++++++++ .../0020_add-apiproject-proxy-model.py | 24 +++++++++++++++++++ 2 files changed, 48 insertions(+) create mode 100644 readthedocs/builds/migrations/0004_add-apiversion-proxy-model.py create mode 100644 readthedocs/projects/migrations/0020_add-apiproject-proxy-model.py diff --git a/readthedocs/builds/migrations/0004_add-apiversion-proxy-model.py b/readthedocs/builds/migrations/0004_add-apiversion-proxy-model.py new file mode 100644 index 00000000000..b96db28e95a --- /dev/null +++ b/readthedocs/builds/migrations/0004_add-apiversion-proxy-model.py @@ -0,0 +1,24 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.9.12 on 2017-10-27 00:17 +from __future__ import unicode_literals + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('builds', '0003_add-cold-storage'), + ] + + operations = [ + migrations.CreateModel( + name='APIVersion', + fields=[ + ], + options={ + 'proxy': True, + }, + bases=('builds.version',), + ), + ] diff --git a/readthedocs/projects/migrations/0020_add-apiproject-proxy-model.py b/readthedocs/projects/migrations/0020_add-apiproject-proxy-model.py new file mode 100644 index 00000000000..2bfc29ead31 --- /dev/null +++ b/readthedocs/projects/migrations/0020_add-apiproject-proxy-model.py @@ -0,0 +1,24 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.9.12 on 2017-10-27 00:16 +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('projects', '0019_add-features'), + ] + + operations = [ + migrations.CreateModel( + name='APIProject', + fields=[ + ], + options={ + 'proxy': True, + }, + bases=('projects.project',), + ), + ] From 954f6324388bde66aa3cbac80ed8bfd61bd66f9c Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Thu, 26 Oct 2017 23:27:24 -0600 Subject: [PATCH 04/15] Add some admin features --- readthedocs/projects/admin.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/readthedocs/projects/admin.py b/readthedocs/projects/admin.py index 244173b1d4e..f62ddb01b9d 100644 --- a/readthedocs/projects/admin.py +++ b/readthedocs/projects/admin.py @@ -184,6 +184,8 @@ class DomainAdmin(admin.ModelAdmin): class FeatureAdmin(admin.ModelAdmin): model = Feature form = FeatureForm + list_display = ('project', 'feature') + search_fields = ('project__name', 'project__slug', 'feature') admin.site.register(Project, ProjectAdmin) From ae8ec3d4569af530fb96a29b9e8bd43390fa1e36 Mon Sep 17 00:00:00 2001 From: Ben Hearsum Date: Tue, 17 Oct 2017 16:44:00 -0400 Subject: [PATCH 05/15] Bump setuptools version. --- readthedocs/doc_builder/python_environments.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readthedocs/doc_builder/python_environments.py b/readthedocs/doc_builder/python_environments.py index 707df75ca02..e11bd87ef77 100644 --- a/readthedocs/doc_builder/python_environments.py +++ b/readthedocs/doc_builder/python_environments.py @@ -117,7 +117,7 @@ def install_core_requirements(self): """Install basic Read the Docs requirements into the virtualenv.""" requirements = [ 'Pygments==2.2.0', - 'setuptools==28.8.0', + 'setuptools==36.6.0', 'docutils==0.13.1', 'mock==1.0.1', 'pillow==2.6.1', From 59f681f9b44f90d77cfe6e7a382c255883e2d150 Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Fri, 27 Oct 2017 00:18:11 -0600 Subject: [PATCH 06/15] Add feature flip for setuptools latest version Also, add a convenience method on the Project model for feature flag dependent values. --- readthedocs/doc_builder/python_environments.py | 15 ++++++++++----- readthedocs/projects/models.py | 10 ++++++++++ 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/readthedocs/doc_builder/python_environments.py b/readthedocs/doc_builder/python_environments.py index e11bd87ef77..a95339fecb9 100644 --- a/readthedocs/doc_builder/python_environments.py +++ b/readthedocs/doc_builder/python_environments.py @@ -117,7 +117,11 @@ def install_core_requirements(self): """Install basic Read the Docs requirements into the virtualenv.""" requirements = [ 'Pygments==2.2.0', - 'setuptools==36.6.0', + self.project.get_feature_value( + Feature.USE_SETUPTOOLS_LATEST, + 'setuptools<36.7', + 'setuptools==28.8.0', + ), 'docutils==0.13.1', 'mock==1.0.1', 'pillow==2.6.1', @@ -129,11 +133,12 @@ def install_core_requirements(self): if self.project.documentation_type == 'mkdocs': requirements.append('mkdocs==0.15.0') else: - if self.project.has_feature(Feature.USE_SPHINX_LATEST): - requirements.append('sphinx<1.7') - else: - requirements.append('sphinx==1.5.6') requirements.extend([ + self.project.get_feature_value( + Feature.USE_SPHINX_LATEST, + 'sphinx<1.7', + 'sphinx==1.5.6', + ), 'sphinx-rtd-theme<0.3', 'readthedocs-sphinx-ext<0.6' ]) diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index 90aac9e80fa..21a129a7159 100644 --- a/readthedocs/projects/models.py +++ b/readthedocs/projects/models.py @@ -825,6 +825,14 @@ def has_feature(self, feature): """Does project have existing feature flag""" return self.features.filter(feature=feature).exists() + def get_feature_value(self, feature, positive, negative): + """Look up project feature, return corresponding value + + If a project has a feature, return ``positive``, otherwise return + ``negative`` + """ + return positive if self.has_feature(feature) else negative + class APIProject(Project): @@ -982,9 +990,11 @@ def add_features(sender, **kwargs): # Feature constants - this is not a exhaustive list of features, features # may be added by other packages USE_SPHINX_LATEST = 'use_sphinx_latest' + USE_SETUPTOOLS_LATEST = 'use_setuptools_latest' FEATURES = ( (USE_SPHINX_LATEST, _('Use latest version of Sphinx')), + (USE_SETUPTOOLS_LATEST, _('Use latest version of setuptools')), ) project = models.ForeignKey( From 889cf0b273ee3929e06182f08e0fa9bcc7ab892e Mon Sep 17 00:00:00 2001 From: Will Mayner Date: Wed, 30 Aug 2017 22:23:06 -0500 Subject: [PATCH 07/15] Use `--upgrade` instead of `-U` in `pip` args --- readthedocs/doc_builder/python_environments.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readthedocs/doc_builder/python_environments.py b/readthedocs/doc_builder/python_environments.py index a95339fecb9..98723508190 100644 --- a/readthedocs/doc_builder/python_environments.py +++ b/readthedocs/doc_builder/python_environments.py @@ -148,7 +148,7 @@ def install_core_requirements(self): self.venv_bin(filename='pip'), 'install', '--use-wheel', - '-U', + '--upgrade', '--cache-dir', self.project.pip_cache_path, ] From 313ef521bff2b6a27df0654858b4cc418e55fa77 Mon Sep 17 00:00:00 2001 From: Will Mayner Date: Wed, 30 Aug 2017 22:24:13 -0500 Subject: [PATCH 08/15] Add upgrade flag to user environment pip install Installs the user-provided requirements with the `--upgrade` flag. --- readthedocs/doc_builder/python_environments.py | 1 + 1 file changed, 1 insertion(+) diff --git a/readthedocs/doc_builder/python_environments.py b/readthedocs/doc_builder/python_environments.py index 98723508190..a6a81b10345 100644 --- a/readthedocs/doc_builder/python_environments.py +++ b/readthedocs/doc_builder/python_environments.py @@ -182,6 +182,7 @@ def install_user_requirements(self): 'python', self.venv_bin(filename='pip'), 'install', + '--upgrade', '--exists-action=w', '--cache-dir', self.project.pip_cache_path, From 8d6c673074170a38096d1a741d384a8c49465469 Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Fri, 27 Oct 2017 00:36:49 -0600 Subject: [PATCH 09/15] Add feature flip for always upgrading user defined packages on pip install We're not yet sure if this will blow up user defined requirements, feature here will be used to test on a sample. Refs #3077 Closes #3077 --- readthedocs/doc_builder/python_environments.py | 10 ++++++++-- readthedocs/projects/models.py | 2 ++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/readthedocs/doc_builder/python_environments.py b/readthedocs/doc_builder/python_environments.py index a6a81b10345..6eaea8b724b 100644 --- a/readthedocs/doc_builder/python_environments.py +++ b/readthedocs/doc_builder/python_environments.py @@ -178,15 +178,21 @@ def install_user_requirements(self): break if requirements_file_path: - self.build_env.run( + args = [ 'python', self.venv_bin(filename='pip'), 'install', - '--upgrade', + ] + if self.project.has_feature(Feature.PIP_ALWAYS_UPGRADE): + args += ['--upgrade'] + args += [ '--exists-action=w', '--cache-dir', self.project.pip_cache_path, '-r{0}'.format(requirements_file_path), + ] + self.build_env.run( + *args, cwd=self.checkout_path, bin_path=self.venv_bin() ) diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index 21a129a7159..42a92739a6b 100644 --- a/readthedocs/projects/models.py +++ b/readthedocs/projects/models.py @@ -991,10 +991,12 @@ def add_features(sender, **kwargs): # may be added by other packages USE_SPHINX_LATEST = 'use_sphinx_latest' USE_SETUPTOOLS_LATEST = 'use_setuptools_latest' + PIP_ALWAYS_UPGRADE = 'pip_always_upgrade' FEATURES = ( (USE_SPHINX_LATEST, _('Use latest version of Sphinx')), (USE_SETUPTOOLS_LATEST, _('Use latest version of setuptools')), + (PIP_ALWAYS_UPGRADE, _('Always run pip install --upgrade')), ) project = models.ForeignKey( From e6d2bdc30390d8b2b66688fe30759522688c5493 Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Fri, 27 Oct 2017 13:09:45 -0600 Subject: [PATCH 10/15] Rework feature relationship, add default true value based on date This allows for features on deprecated code, where we can say the feature is true historically. --- readthedocs/projects/admin.py | 9 +++-- readthedocs/projects/forms.py | 2 +- .../projects/migrations/0019_add-features.py | 11 +++--- ...model.py => 0020_add-api-project-proxy.py} | 2 +- readthedocs/projects/models.py | 34 +++++++++++++++---- readthedocs/projects/querysets.py | 11 ++++++ readthedocs/rtd_tests/tests/test_api.py | 20 ++++++++++- .../rtd_tests/tests/test_project_querysets.py | 31 ++++++++++++++++- 8 files changed, 102 insertions(+), 18 deletions(-) rename readthedocs/projects/migrations/{0020_add-apiproject-proxy-model.py => 0020_add-api-project-proxy.py} (90%) diff --git a/readthedocs/projects/admin.py b/readthedocs/projects/admin.py index f62ddb01b9d..810c9dc215a 100644 --- a/readthedocs/projects/admin.py +++ b/readthedocs/projects/admin.py @@ -184,8 +184,13 @@ class DomainAdmin(admin.ModelAdmin): class FeatureAdmin(admin.ModelAdmin): model = Feature form = FeatureForm - list_display = ('project', 'feature') - search_fields = ('project__name', 'project__slug', 'feature') + list_display = ('feature', 'foo') + search_fields = ('feature',) + filter_horizontal = ('projects',) + readonly_fields = ('add_date',) + + def foo(self): + return 'x' admin.site.register(Project, ProjectAdmin) diff --git a/readthedocs/projects/forms.py b/readthedocs/projects/forms.py index a5275705701..ccc7bda44fc 100644 --- a/readthedocs/projects/forms.py +++ b/readthedocs/projects/forms.py @@ -610,7 +610,7 @@ class FeatureForm(forms.ModelForm): class Meta(object): model = Feature - fields = ['project', 'feature'] + fields = ['projects', 'feature', 'default_true'] def __init__(self, *args, **kwargs): super(FeatureForm, self).__init__(*args, **kwargs) diff --git a/readthedocs/projects/migrations/0019_add-features.py b/readthedocs/projects/migrations/0019_add-features.py index 7d15b159fe1..910734a7d99 100644 --- a/readthedocs/projects/migrations/0019_add-features.py +++ b/readthedocs/projects/migrations/0019_add-features.py @@ -1,9 +1,8 @@ # -*- coding: utf-8 -*- -# Generated by Django 1.9.12 on 2017-10-26 14:40 +# Generated by Django 1.9.12 on 2017-10-27 12:55 from __future__ import unicode_literals from django.db import migrations, models -import django.db.models.deletion class Migration(migrations.Migration): @@ -17,12 +16,14 @@ class Migration(migrations.Migration): name='Feature', fields=[ ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), - ('feature', models.CharField(max_length=32, verbose_name='Project feature')), + ('feature', models.CharField(max_length=32, unique=True, verbose_name='Feature identifier')), + ('add_date', models.DateTimeField(auto_now_add=True, verbose_name='Date feature was added')), + ('default_true', models.BooleanField(default=False, verbose_name='Historical default is True')), ], ), migrations.AddField( model_name='feature', - name='project', - field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='features', to='projects.Project'), + name='projects', + field=models.ManyToManyField(blank=True, to='projects.Project'), ), ] diff --git a/readthedocs/projects/migrations/0020_add-apiproject-proxy-model.py b/readthedocs/projects/migrations/0020_add-api-project-proxy.py similarity index 90% rename from readthedocs/projects/migrations/0020_add-apiproject-proxy-model.py rename to readthedocs/projects/migrations/0020_add-api-project-proxy.py index 2bfc29ead31..34eafa4846f 100644 --- a/readthedocs/projects/migrations/0020_add-apiproject-proxy-model.py +++ b/readthedocs/projects/migrations/0020_add-api-project-proxy.py @@ -1,5 +1,5 @@ # -*- coding: utf-8 -*- -# Generated by Django 1.9.12 on 2017-10-27 00:16 +# Generated by Django 1.9.12 on 2017-10-27 12:56 from __future__ import unicode_literals from django.db import migrations, models diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index 90aac9e80fa..d67d06f9fe6 100644 --- a/readthedocs/projects/models.py +++ b/readthedocs/projects/models.py @@ -26,7 +26,8 @@ from readthedocs.projects.querysets import ( ProjectQuerySet, RelatedProjectQuerySet, - ChildRelatedProjectQuerySet + ChildRelatedProjectQuerySet, + FeatureQuerySet, ) from readthedocs.projects.templatetags.projects_tags import sort_version_aware from readthedocs.projects.version_handling import determine_stable_version, version_windows @@ -821,8 +822,17 @@ def add_comment(self, version_slug, page, content_hash, commit, user, text): hash=content_hash, commit=commit) return node.comments.create(user=user, text=text) + @property + def features(self): + return Feature.objects.for_project(self) + def has_feature(self, feature): - """Does project have existing feature flag""" + """Does project have existing feature flag + + If the feature has a historical True value before the feature was added, + we consider the project to have the flag. This is used for deprecating a + feature or changing behavior for new projects + """ return self.features.filter(feature=feature).exists() @@ -987,21 +997,31 @@ def add_features(sender, **kwargs): (USE_SPHINX_LATEST, _('Use latest version of Sphinx')), ) - project = models.ForeignKey( + projects = models.ManyToManyField( Project, - related_name='features', + blank=True, ) # Feature is not implemented as a ChoiceField, as we don't want validation # at the database level on this field. Arbitrary values are allowed here. feature = models.CharField( - _('Project feature'), + _('Feature identifier'), max_length=32, + unique=True, ) + add_date = models.DateTimeField( + _('Date feature was added'), + auto_now_add=True, + ) + default_true = models.BooleanField( + _('Historical default is True'), + default=False, + ) + + objects = FeatureQuerySet.as_manager() def __str__(self): - return "{0} feature for {1}".format( + return "{0} feature".format( self.get_feature_display(), - self.project, ) def get_feature_display(self): diff --git a/readthedocs/projects/querysets.py b/readthedocs/projects/querysets.py index 5f7688d5529..8c95041c163 100644 --- a/readthedocs/projects/querysets.py +++ b/readthedocs/projects/querysets.py @@ -3,6 +3,7 @@ from __future__ import absolute_import from django.db import models +from django.db.models import Q from guardian.shortcuts import get_objects_for_user from . import constants @@ -148,3 +149,13 @@ class ChildRelatedProjectQuerySetBase(RelatedProjectQuerySetBase): class ChildRelatedProjectQuerySet(SettingsOverrideObject): _default_class = ChildRelatedProjectQuerySetBase _override_setting = 'RELATED_PROJECT_MANAGER' + + +class FeatureQuerySet(models.QuerySet): + use_for_related_fields = True + + def for_project(self, project): + return self.filter( + Q(projects=project) | + Q(default_true=True, add_date__gt=project.pub_date) + ) diff --git a/readthedocs/rtd_tests/tests/test_api.py b/readthedocs/rtd_tests/tests/test_api.py index bc2e672f870..75f3dc00ab7 100644 --- a/readthedocs/rtd_tests/tests/test_api.py +++ b/readthedocs/rtd_tests/tests/test_api.py @@ -15,7 +15,7 @@ from readthedocs.builds.models import Build, Version from readthedocs.integrations.models import Integration -from readthedocs.projects.models import Project +from readthedocs.projects.models import Project, Feature from readthedocs.oauth.models import RemoteRepository, RemoteOrganization @@ -227,6 +227,24 @@ def test_ensure_get_unauth(self): resp = self.client.get("/api/v1/project/", data={"format": "json"}) self.assertEqual(resp.status_code, 200) + def test_project_features(self): + user = get(User, is_staff=True) + project = get(Project, main_language_project=None) + # One explicit, one implicit feature + feature1 = get(Feature, projects=[project]) + feature2 = get(Feature, projects=[], default_true=True) + feature3 = get(Feature, projects=[], default_true=False) + client = APIClient() + + client.force_authenticate(user=user) + resp = client.get('/api/v2/project/%s/' % (project.pk)) + self.assertEqual(resp.status_code, 200) + self.assertIn('features', resp.data) + self.assertEqual( + resp.data['features'], + [feature1.feature, feature2.feature] + ) + class APIImportTests(TestCase): diff --git a/readthedocs/rtd_tests/tests/test_project_querysets.py b/readthedocs/rtd_tests/tests/test_project_querysets.py index 237af53c3d4..c729b830fbe 100644 --- a/readthedocs/rtd_tests/tests/test_project_querysets.py +++ b/readthedocs/rtd_tests/tests/test_project_querysets.py @@ -1,9 +1,11 @@ from __future__ import absolute_import +from datetime import timedelta + import django_dynamic_fixture as fixture from django.test import TestCase -from readthedocs.projects.models import Project, ProjectRelationship +from readthedocs.projects.models import Project, ProjectRelationship, Feature from readthedocs.projects.querysets import (ParentRelatedProjectQuerySet, ChildRelatedProjectQuerySet) @@ -27,3 +29,30 @@ def test_subproject_queryset_as_manager_gets_correct_class(self): mgr.__class__.__name__, 'ManagerFromParentRelatedProjectQuerySetBase' ) + + +class FeatureQuerySetTests(TestCase): + + def test_feature_for_project_is_explicit_applied(self): + project = fixture.get(Project, main_language_project=None) + feature = fixture.get(Feature, projects=[project]) + self.assertTrue(project.has_feature(feature.feature)) + + def test_feature_for_project_is_implicitly_applied(self): + project = fixture.get(Project, main_language_project=None) + feature1 = fixture.get(Feature, projects=[project]) + feature2 = fixture.get( + Feature, + projects=[], + add_date=project.pub_date + timedelta(days=1), + default_true=False, + ) + feature3 = fixture.get( + Feature, + projects=[], + add_date=project.pub_date + timedelta(days=1), + default_true=True, + ) + self.assertTrue(project.has_feature(feature1.feature)) + self.assertFalse(project.has_feature(feature2.feature)) + self.assertTrue(project.has_feature(feature3.feature)) From adbe72a582bab8b31393cf3bb1c814201f2bc634 Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Fri, 27 Oct 2017 15:08:51 -0600 Subject: [PATCH 11/15] Fix some issues, more tests --- readthedocs/projects/admin.py | 11 ++++++-- readthedocs/projects/querysets.py | 2 +- readthedocs/rtd_tests/tests/test_api.py | 16 +++++++++++ .../rtd_tests/tests/test_project_querysets.py | 28 +++++++++++++++++-- 4 files changed, 50 insertions(+), 7 deletions(-) diff --git a/readthedocs/projects/admin.py b/readthedocs/projects/admin.py index 810c9dc215a..41a675d9b9e 100644 --- a/readthedocs/projects/admin.py +++ b/readthedocs/projects/admin.py @@ -184,13 +184,18 @@ class DomainAdmin(admin.ModelAdmin): class FeatureAdmin(admin.ModelAdmin): model = Feature form = FeatureForm - list_display = ('feature', 'foo') + list_display = ('feature', 'projects_list', 'default_true') search_fields = ('feature',) filter_horizontal = ('projects',) readonly_fields = ('add_date',) - def foo(self): - return 'x' + def projects_list(self, feature): + projects = [p.name for p in feature.projects.all()[0:10]] + if feature.projects.count() > 10: + projects += [ + '... and {0} more'.format(feature.projects.count() - 10) + ] + return ', '.join(projects) admin.site.register(Project, ProjectAdmin) diff --git a/readthedocs/projects/querysets.py b/readthedocs/projects/querysets.py index 8c95041c163..241cd6c929d 100644 --- a/readthedocs/projects/querysets.py +++ b/readthedocs/projects/querysets.py @@ -158,4 +158,4 @@ def for_project(self, project): return self.filter( Q(projects=project) | Q(default_true=True, add_date__gt=project.pub_date) - ) + ).distinct() diff --git a/readthedocs/rtd_tests/tests/test_api.py b/readthedocs/rtd_tests/tests/test_api.py index 75f3dc00ab7..ddc178b5d7b 100644 --- a/readthedocs/rtd_tests/tests/test_api.py +++ b/readthedocs/rtd_tests/tests/test_api.py @@ -245,6 +245,22 @@ def test_project_features(self): [feature1.feature, feature2.feature] ) + def test_project_features_multiple_projects(self): + user = get(User, is_staff=True) + project1 = get(Project, main_language_project=None) + project2 = get(Project, main_language_project=None) + feature = get(Feature, projects=[project1, project2], default_true=True) + client = APIClient() + + client.force_authenticate(user=user) + resp = client.get('/api/v2/project/%s/' % (project1.pk)) + self.assertEqual(resp.status_code, 200) + self.assertIn('features', resp.data) + self.assertEqual( + resp.data['features'], + [feature.feature] + ) + class APIImportTests(TestCase): diff --git a/readthedocs/rtd_tests/tests/test_project_querysets.py b/readthedocs/rtd_tests/tests/test_project_querysets.py index c729b830fbe..662f34787ed 100644 --- a/readthedocs/rtd_tests/tests/test_project_querysets.py +++ b/readthedocs/rtd_tests/tests/test_project_querysets.py @@ -40,19 +40,41 @@ def test_feature_for_project_is_explicit_applied(self): def test_feature_for_project_is_implicitly_applied(self): project = fixture.get(Project, main_language_project=None) + # Explicit feature feature1 = fixture.get(Feature, projects=[project]) + # False implicit feature feature2 = fixture.get( Feature, projects=[], add_date=project.pub_date + timedelta(days=1), default_true=False, ) + # True implicit feature before add date feature3 = fixture.get( Feature, projects=[], add_date=project.pub_date + timedelta(days=1), default_true=True, ) - self.assertTrue(project.has_feature(feature1.feature)) - self.assertFalse(project.has_feature(feature2.feature)) - self.assertTrue(project.has_feature(feature3.feature)) + # True implicit feature after add date + feature4 = fixture.get( + Feature, + projects=[], + add_date=project.pub_date - timedelta(days=1), + default_true=True, + ) + self.assertQuerysetEqual( + Feature.objects.for_project(project), + [repr(feature1), repr(feature3)], + ordered=False, + ) + + def test_feature_multiple_projects(self): + project1 = fixture.get(Project, main_language_project=None) + project2 = fixture.get(Project, main_language_project=None) + feature = fixture.get(Feature, projects=[project1, project2]) + self.assertQuerysetEqual( + Feature.objects.for_project(project1), + [repr(feature)], + ordered=False, + ) From dbdf59f61c732aa1871e43476b98cb919a64c899 Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Mon, 30 Oct 2017 16:21:44 -0600 Subject: [PATCH 12/15] Rename Feature.feature -> Feature.feature_id, other cleanup --- readthedocs/builds/models.py | 4 ++-- readthedocs/doc_builder/python_environments.py | 2 +- readthedocs/projects/admin.py | 13 ++++--------- .../projects/migrations/0019_add-features.py | 2 +- readthedocs/projects/models.py | 14 ++++++++------ readthedocs/restapi/serializers.py | 2 +- readthedocs/rtd_tests/tests/test_api.py | 4 ++-- .../rtd_tests/tests/test_project_querysets.py | 2 +- 8 files changed, 20 insertions(+), 23 deletions(-) diff --git a/readthedocs/builds/models.py b/readthedocs/builds/models.py index 0c738f7992d..f3927178987 100644 --- a/readthedocs/builds/models.py +++ b/readthedocs/builds/models.py @@ -322,8 +322,8 @@ class Meta: def __init__(self, *args, **kwargs): self.project = APIProject(**kwargs.pop('project', {})) - # This was replicated from readthedocs.projects.utils.make_api_project. - # I'm not certain why these need to be deleted + # These fields only exist on the API return, not on the model, so we'll + # remove them to avoid throwing exceptions due to unexpected fields for key in ['resource_uri', 'absolute_url', 'downloads']: try: del kwargs[key] diff --git a/readthedocs/doc_builder/python_environments.py b/readthedocs/doc_builder/python_environments.py index 707df75ca02..39544113ce0 100644 --- a/readthedocs/doc_builder/python_environments.py +++ b/readthedocs/doc_builder/python_environments.py @@ -130,7 +130,7 @@ def install_core_requirements(self): requirements.append('mkdocs==0.15.0') else: if self.project.has_feature(Feature.USE_SPHINX_LATEST): - requirements.append('sphinx<1.7') + requirements.append('sphinx') else: requirements.append('sphinx==1.5.6') requirements.extend([ diff --git a/readthedocs/projects/admin.py b/readthedocs/projects/admin.py index 41a675d9b9e..faa3690e04d 100644 --- a/readthedocs/projects/admin.py +++ b/readthedocs/projects/admin.py @@ -184,18 +184,13 @@ class DomainAdmin(admin.ModelAdmin): class FeatureAdmin(admin.ModelAdmin): model = Feature form = FeatureForm - list_display = ('feature', 'projects_list', 'default_true') - search_fields = ('feature',) + list_display = ('feature_id', 'project_count', 'default_true') + search_fields = ('feature_id',) filter_horizontal = ('projects',) readonly_fields = ('add_date',) - def projects_list(self, feature): - projects = [p.name for p in feature.projects.all()[0:10]] - if feature.projects.count() > 10: - projects += [ - '... and {0} more'.format(feature.projects.count() - 10) - ] - return ', '.join(projects) + def project_count(self, feature): + return feature.projects.count() admin.site.register(Project, ProjectAdmin) diff --git a/readthedocs/projects/migrations/0019_add-features.py b/readthedocs/projects/migrations/0019_add-features.py index 910734a7d99..6d1036dd123 100644 --- a/readthedocs/projects/migrations/0019_add-features.py +++ b/readthedocs/projects/migrations/0019_add-features.py @@ -16,7 +16,7 @@ class Migration(migrations.Migration): name='Feature', fields=[ ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), - ('feature', models.CharField(max_length=32, unique=True, verbose_name='Feature identifier')), + ('feature_id', models.CharField(max_length=32, unique=True, verbose_name='Feature identifier')), ('add_date', models.DateTimeField(auto_now_add=True, verbose_name='Date feature was added')), ('default_true', models.BooleanField(default=False, verbose_name='Historical default is True')), ], diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index d67d06f9fe6..86a63937d22 100644 --- a/readthedocs/projects/models.py +++ b/readthedocs/projects/models.py @@ -826,14 +826,14 @@ def add_comment(self, version_slug, page, content_hash, commit, user, text): def features(self): return Feature.objects.for_project(self) - def has_feature(self, feature): + def has_feature(self, feature_id): """Does project have existing feature flag If the feature has a historical True value before the feature was added, we consider the project to have the flag. This is used for deprecating a feature or changing behavior for new projects """ - return self.features.filter(feature=feature).exists() + return self.features.filter(feature_id=feature_id).exists() class APIProject(Project): @@ -857,6 +857,8 @@ class Meta: def __init__(self, *args, **kwargs): self.features = kwargs.pop('features', []) + # These fields only exist on the API return, not on the model, so we'll + # remove them to avoid throwing exceptions due to unexpected fields for key in ['users', 'resource_uri', 'absolute_url', 'downloads', 'main_language_project', 'related_projects']: try: @@ -868,8 +870,8 @@ def __init__(self, *args, **kwargs): def save(self, *args, **kwargs): return 0 - def has_feature(self, feature): - return feature in self.features + def has_feature(self, feature_id): + return feature_id in self.features @python_2_unicode_compatible @@ -1003,7 +1005,7 @@ def add_features(sender, **kwargs): ) # Feature is not implemented as a ChoiceField, as we don't want validation # at the database level on this field. Arbitrary values are allowed here. - feature = models.CharField( + feature_id = models.CharField( _('Feature identifier'), max_length=32, unique=True, @@ -1030,4 +1032,4 @@ def get_feature_display(self): Because the field is not a ChoiceField here, we need to manually implement this behavior. """ - return dict(self.FEATURES).get(self.feature, self.feature) + return dict(self.FEATURES).get(self.feature_id, self.feature_id) diff --git a/readthedocs/restapi/serializers.py b/readthedocs/restapi/serializers.py index 481e04c20dc..b85f2373136 100644 --- a/readthedocs/restapi/serializers.py +++ b/readthedocs/restapi/serializers.py @@ -37,7 +37,7 @@ class ProjectAdminSerializer(ProjectSerializer): features = serializers.SlugRelatedField( many=True, read_only=True, - slug_field='feature', + slug_field='feature_id', ) class Meta(ProjectSerializer.Meta): diff --git a/readthedocs/rtd_tests/tests/test_api.py b/readthedocs/rtd_tests/tests/test_api.py index ddc178b5d7b..81d7b05eaaa 100644 --- a/readthedocs/rtd_tests/tests/test_api.py +++ b/readthedocs/rtd_tests/tests/test_api.py @@ -242,7 +242,7 @@ def test_project_features(self): self.assertIn('features', resp.data) self.assertEqual( resp.data['features'], - [feature1.feature, feature2.feature] + [feature1.feature_id, feature2.feature_id] ) def test_project_features_multiple_projects(self): @@ -258,7 +258,7 @@ def test_project_features_multiple_projects(self): self.assertIn('features', resp.data) self.assertEqual( resp.data['features'], - [feature.feature] + [feature.feature_id] ) diff --git a/readthedocs/rtd_tests/tests/test_project_querysets.py b/readthedocs/rtd_tests/tests/test_project_querysets.py index 662f34787ed..3772c6688ee 100644 --- a/readthedocs/rtd_tests/tests/test_project_querysets.py +++ b/readthedocs/rtd_tests/tests/test_project_querysets.py @@ -36,7 +36,7 @@ class FeatureQuerySetTests(TestCase): def test_feature_for_project_is_explicit_applied(self): project = fixture.get(Project, main_language_project=None) feature = fixture.get(Feature, projects=[project]) - self.assertTrue(project.has_feature(feature.feature)) + self.assertTrue(project.has_feature(feature.feature_id)) def test_feature_for_project_is_implicitly_applied(self): project = fixture.get(Project, main_language_project=None) From c4b0cacadb650d7ca99285d762750bbdc8018b06 Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Mon, 30 Oct 2017 16:38:22 -0600 Subject: [PATCH 13/15] Drop setuptools pinning --- readthedocs/doc_builder/python_environments.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readthedocs/doc_builder/python_environments.py b/readthedocs/doc_builder/python_environments.py index c39e6e27301..4690579ed80 100644 --- a/readthedocs/doc_builder/python_environments.py +++ b/readthedocs/doc_builder/python_environments.py @@ -119,7 +119,7 @@ def install_core_requirements(self): 'Pygments==2.2.0', self.project.get_feature_value( Feature.USE_SETUPTOOLS_LATEST, - 'setuptools<36.7', + 'setuptools', 'setuptools==28.8.0', ), 'docutils==0.13.1', From 83b0a2fccbf23d8a9254429d48bf52c28fa89d08 Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Mon, 30 Oct 2017 23:46:00 -0600 Subject: [PATCH 14/15] Use semver for setuptools version --- readthedocs/doc_builder/python_environments.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/readthedocs/doc_builder/python_environments.py b/readthedocs/doc_builder/python_environments.py index b8e84bde6db..e537b98c9f9 100644 --- a/readthedocs/doc_builder/python_environments.py +++ b/readthedocs/doc_builder/python_environments.py @@ -117,9 +117,11 @@ def install_core_requirements(self): """Install basic Read the Docs requirements into the virtualenv.""" requirements = [ 'Pygments==2.2.0', + # Assume semver for setuptools version, support up to next backwards + # incompatible release self.project.get_feature_value( Feature.USE_SETUPTOOLS_LATEST, - 'setuptools', + 'setuptools<37', 'setuptools==28.8.0', ), 'docutils==0.13.1', From 5b185ccfc0c87c851d345065fb0103889619339d Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Mon, 30 Oct 2017 23:53:36 -0600 Subject: [PATCH 15/15] Missed merge conflict --- readthedocs/projects/models.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index ac5bc9bee7f..068c3622675 100644 --- a/readthedocs/projects/models.py +++ b/readthedocs/projects/models.py @@ -1002,17 +1002,11 @@ def add_features(sender, **kwargs): # Feature constants - this is not a exhaustive list of features, features # may be added by other packages USE_SPHINX_LATEST = 'use_sphinx_latest' -<<<<<<< HEAD USE_SETUPTOOLS_LATEST = 'use_setuptools_latest' FEATURES = ( (USE_SPHINX_LATEST, _('Use latest version of Sphinx')), (USE_SETUPTOOLS_LATEST, _('Use latest version of setuptools')), -======= - - FEATURES = ( - (USE_SPHINX_LATEST, _('Use latest version of Sphinx')), ->>>>>>> master ) projects = models.ManyToManyField(