From d4762d9d0f799c5ebda2d241e0934a7d88e30739 Mon Sep 17 00:00:00 2001 From: Fatema Boxwala Date: Thu, 2 Jun 2016 18:40:45 -0500 Subject: [PATCH] pep257ifys core and api (#2245) --- prospector.yml | 4 ++++ readthedocs/api/utils.py | 18 ++++++++++++------ readthedocs/core/forms.py | 18 ++++++++++-------- .../core/management/commands/archive.py | 7 +++++-- .../core/management/commands/clean_builds.py | 4 +--- .../commands/reindex_elasticsearch.py | 4 +--- .../core/management/commands/sync_builds.py | 7 +++++-- .../core/management/commands/update_api.py | 1 + .../core/management/commands/update_repos.py | 6 ++++-- .../management/commands/update_versions.py | 7 +++++-- readthedocs/core/middleware.py | 2 ++ readthedocs/core/mixins.py | 4 +--- readthedocs/core/models.py | 4 ++-- readthedocs/core/symlink.py | 1 + readthedocs/core/templatetags/core_tags.py | 5 +++-- readthedocs/core/utils/__init__.py | 4 +--- readthedocs/core/utils/tasks/public.py | 15 +++++++-------- readthedocs/core/utils/tasks/retrieve.py | 2 +- readthedocs/core/views/__init__.py | 14 +++++++------- readthedocs/core/views/hooks.py | 8 ++------ readthedocs/core/views/serve.py | 4 +--- 21 files changed, 76 insertions(+), 63 deletions(-) diff --git a/prospector.yml b/prospector.yml index 21138fc6c..fa5412c87 100644 --- a/prospector.yml +++ b/prospector.yml @@ -34,6 +34,10 @@ mccabe: pep257: run: false + disable: + - D105 + - D211 + - D104 pyflakes: disable: diff --git a/readthedocs/api/utils.py b/readthedocs/api/utils.py index 56ec26d94..160ad2fee 100644 --- a/readthedocs/api/utils.py +++ b/readthedocs/api/utils.py @@ -19,13 +19,16 @@ log = logging.getLogger(__name__) class SearchMixin(object): - ''' + + """ Adds a search api to any ModelResource provided the model is indexed. + The search can be configured using the Meta class in each ModelResource. The search is limited to the model defined by the meta queryset. If the search is invalid, a 400 Bad Request will be raised. e.g. + class Meta: # Return facet counts for each facetname search_facets = ['facetname1', 'facetname1'] @@ -35,7 +38,8 @@ class SearchMixin(object): # Highlight search terms in the text search_highlight = True - ''' + """ + def get_search(self, request, **kwargs): self.method_check(request, allowed=['get']) self.is_authenticated(request) @@ -49,11 +53,12 @@ class SearchMixin(object): return self.create_response(request, object_list) def _url_template(self, query, selected_facets): - ''' + """ Construct a url template to assist with navigating the resources. + This looks a bit nasty but urllib.urlencode resulted in even nastier output... - ''' + """ query_params = [] for facet in selected_facets: query_params.append(('selected_facets', facet)) @@ -67,12 +72,13 @@ class SearchMixin(object): def _search(self, request, model, facets=None, page_size=20, highlight=True): - ''' + """ `facets` + A list of facets to include with the results `models` Limit the search to one or more models - ''' + """ form = FacetedSearchForm(request.GET, facets=facets or [], models=(model,), load_all=True) if not form.is_valid(): diff --git a/readthedocs/core/forms.py b/readthedocs/core/forms.py index bdaabb10f..da2dd7e98 100644 --- a/readthedocs/core/forms.py +++ b/readthedocs/core/forms.py @@ -42,17 +42,19 @@ class UserProfileForm(forms.ModelForm): class FacetField(forms.MultipleChoiceField): - ''' - For filtering searches on a facet, with validation for the format - of facet values. - ''' + """ + For filtering searches on a facet. + + Has validation for the format of facet values. + """ def valid_value(self, value): - ''' + """ Although this is a choice field, no choices need to be supplied. + Instead, we just validate that the value is in the correct format for facet filtering (facet_name:value) - ''' + """ if ":" not in value: return False return True @@ -60,14 +62,14 @@ class FacetField(forms.MultipleChoiceField): class FacetedSearchForm(SearchForm): - ''' + """ Supports fetching faceted results with a corresponding query. `facets` A list of facet names for which to get facet counts `models` Limit the search to one or more models - ''' + """ selected_facets = FacetField(required=False) diff --git a/readthedocs/core/management/commands/archive.py b/readthedocs/core/management/commands/archive.py index a9f2c8dad..855044b47 100644 --- a/readthedocs/core/management/commands/archive.py +++ b/readthedocs/core/management/commands/archive.py @@ -10,8 +10,11 @@ log = logging.getLogger(__name__) class Command(BaseCommand): - """Custom management command to rebuild documentation for all projects on - the site. Invoked via ``./manage.py update_repos``. + + """ + Custom management command to rebuild documentation for all projects. + + Invoked via ``./manage.py update_repos``. """ def handle(self, *args, **options): diff --git a/readthedocs/core/management/commands/clean_builds.py b/readthedocs/core/management/commands/clean_builds.py index dc3d0c7a4..89164c2a2 100644 --- a/readthedocs/core/management/commands/clean_builds.py +++ b/readthedocs/core/management/commands/clean_builds.py @@ -27,9 +27,7 @@ class Command(BaseCommand): ) def handle(self, *args, **options): - ''' - Find stale builds and remove build paths - ''' + """Find stale builds and remove build paths""" max_date = datetime.now() - timedelta(days=options['days']) queryset = (Build.objects .values('project', 'version') diff --git a/readthedocs/core/management/commands/reindex_elasticsearch.py b/readthedocs/core/management/commands/reindex_elasticsearch.py index 83ba626bd..c55f185eb 100644 --- a/readthedocs/core/management/commands/reindex_elasticsearch.py +++ b/readthedocs/core/management/commands/reindex_elasticsearch.py @@ -22,9 +22,7 @@ class Command(BaseCommand): ) def handle(self, *args, **options): - ''' - Build/index all versions or a single project's version - ''' + """Build/index all versions or a single project's version""" project = options['project'] queryset = Version.objects.public() diff --git a/readthedocs/core/management/commands/sync_builds.py b/readthedocs/core/management/commands/sync_builds.py index 251fa0f6d..b0928551c 100644 --- a/readthedocs/core/management/commands/sync_builds.py +++ b/readthedocs/core/management/commands/sync_builds.py @@ -9,8 +9,11 @@ log = logging.getLogger(__name__) class Command(BaseCommand): - """Custom management command to rebuild documentation for all projects on - the site. Invoked via ``./manage.py update_repos``. + + """ + Custom management command to rebuild documentation for all projects. + + Invoked via ``./manage.py update_repos``. """ option_list = BaseCommand.option_list + ( diff --git a/readthedocs/core/management/commands/update_api.py b/readthedocs/core/management/commands/update_api.py index 093ee4e3b..8a3100b23 100644 --- a/readthedocs/core/management/commands/update_api.py +++ b/readthedocs/core/management/commands/update_api.py @@ -9,6 +9,7 @@ log = logging.getLogger(__name__) class Command(BaseCommand): + """ Build documentation using the API and not hitting a database. diff --git a/readthedocs/core/management/commands/update_repos.py b/readthedocs/core/management/commands/update_repos.py index a5b1de67a..4c68eac88 100644 --- a/readthedocs/core/management/commands/update_repos.py +++ b/readthedocs/core/management/commands/update_repos.py @@ -12,8 +12,10 @@ log = logging.getLogger(__name__) class Command(BaseCommand): - """Custom management command to rebuild documentation for all projects on - the site. Invoked via ``./manage.py update_repos``. + """ + Custom management command to rebuild documentation for all projects. + + Invoked via ``./manage.py update_repos``. """ option_list = BaseCommand.option_list + ( diff --git a/readthedocs/core/management/commands/update_versions.py b/readthedocs/core/management/commands/update_versions.py index 31c5374d2..0ae1a6f04 100644 --- a/readthedocs/core/management/commands/update_versions.py +++ b/readthedocs/core/management/commands/update_versions.py @@ -5,8 +5,11 @@ from readthedocs.projects.tasks import update_docs class Command(BaseCommand): - """Custom management command to rebuild documentation for all projects on - the site. Invoked via ``./manage.py update_repos``. + + """ + Custom management command to rebuild documentation for all projects. + + Invoked via ``./manage.py update_repos``. """ def handle(self, *args, **options): diff --git a/readthedocs/core/middleware.py b/readthedocs/core/middleware.py index 86a8e677d..ce8c28d8a 100644 --- a/readthedocs/core/middleware.py +++ b/readthedocs/core/middleware.py @@ -178,6 +178,7 @@ class ProxyMiddleware(object): """ Middleware that sets REMOTE_ADDR based on HTTP_X_FORWARDED_FOR, if the + latter is set. This is useful if you're sitting behind a reverse proxy that causes each request's REMOTE_ADDR to be set to 127.0.0.1. Note that this does NOT validate HTTP_X_FORWARDED_FOR. If you're not behind @@ -207,6 +208,7 @@ class FooterNoSessionMiddleware(SessionMiddleware): This will reduce the size of our session table drastically. """ + IGNORE_URLS = ['/api/v2/footer_html', '/sustainability/view', '/sustainability/click'] def process_request(self, request): diff --git a/readthedocs/core/mixins.py b/readthedocs/core/mixins.py index a6ebdae96..66d114cd5 100644 --- a/readthedocs/core/mixins.py +++ b/readthedocs/core/mixins.py @@ -1,6 +1,4 @@ -""" -Common mixin classes for views -""" +"""Common mixin classes for views""" from vanilla import ListView from django.contrib.auth.decorators import login_required diff --git a/readthedocs/core/models.py b/readthedocs/core/models.py index e1b033e4b..99c2c9f31 100644 --- a/readthedocs/core/models.py +++ b/readthedocs/core/models.py @@ -14,8 +14,8 @@ log = logging.getLogger(__name__) class UserProfile (models.Model): - """Additional information about a User. - """ + """Additional information about a User.""" + user = AutoOneToOneField('auth.User', verbose_name=_('User'), related_name='profile') whitelisted = models.BooleanField(_('Whitelisted'), default=False) diff --git a/readthedocs/core/symlink.py b/readthedocs/core/symlink.py index bb3eb58ca..ef66e9a51 100644 --- a/readthedocs/core/symlink.py +++ b/readthedocs/core/symlink.py @@ -69,6 +69,7 @@ log = logging.getLogger(__name__) class Symlink(object): + """Base class for symlinking of projects.""" def __init__(self, project): diff --git a/readthedocs/core/templatetags/core_tags.py b/readthedocs/core/templatetags/core_tags.py index 1c07fd272..86e6b947e 100644 --- a/readthedocs/core/templatetags/core_tags.py +++ b/readthedocs/core/templatetags/core_tags.py @@ -14,10 +14,11 @@ register = template.Library() @register.filter def gravatar(email, size=48): - """hacked from djangosnippets.org, but basically given an email address + """ + hacked from djangosnippets.org, but basically given an email address + render an img tag with the hashed up bits needed for leetness omgwtfstillreading - """ url = "http://www.gravatar.com/avatar.php?%s" % urllib.urlencode({ 'gravatar_id': hashlib.md5(email).hexdigest(), diff --git a/readthedocs/core/utils/__init__.py b/readthedocs/core/utils/__init__.py index 2683ecb61..367a2c848 100644 --- a/readthedocs/core/utils/__init__.py +++ b/readthedocs/core/utils/__init__.py @@ -16,9 +16,7 @@ SYNC_USER = getattr(settings, 'SYNC_USER', getpass.getuser()) def run_on_app_servers(command): - """ - A helper to copy a single file across app servers - """ + """A helper to copy a single file across app servers""" log.info("Running %s on app servers" % command) ret_val = 0 if getattr(settings, "MULTIPLE_APP_SERVERS", None): diff --git a/readthedocs/core/utils/tasks/public.py b/readthedocs/core/utils/tasks/public.py index 186ff105d..f9fcd0e5e 100644 --- a/readthedocs/core/utils/tasks/public.py +++ b/readthedocs/core/utils/tasks/public.py @@ -14,25 +14,22 @@ STATUS_UPDATES_ENABLED = not getattr(settings, 'CELERY_ALWAYS_EAGER', False) class PublicTask(Task): + """ See oauth.tasks for usage example. Subclasses need to define a ``run_public`` method. """ + public_name = 'unknown' @classmethod def check_permission(cls, request, state, context): - """ - Override this method to define who can monitor this task. - """ + """Override this method to define who can monitor this task.""" return False def get_task_data(self): - """ - Return a tuple with the state that should be set next and the results - task. - """ + """Return tuple with state to be set next and results task.""" state = 'STARTED' info = { 'task_name': self.name, @@ -49,6 +46,7 @@ class PublicTask(Task): def set_permission_context(self, context): """ Set data that can be used by ``check_permission`` to authorize a + request for the this task. By default it will be the ``kwargs`` passed into the task. """ @@ -58,6 +56,7 @@ class PublicTask(Task): def set_public_data(self, data): """ Set data that can be displayed in the frontend to authorized users. + This might include progress data about the task. """ self.request.update(public_data=data) @@ -83,6 +82,7 @@ class PublicTask(Task): def permission_check(check): """ Class decorator for subclasses of PublicTask to sprinkle in re-usable + permission checks:: @permission_check(user_id_matches) @@ -90,7 +90,6 @@ def permission_check(check): def run_public(self, user_id): pass """ - def decorator(cls): cls.check_permission = staticmethod(check) return cls diff --git a/readthedocs/core/utils/tasks/retrieve.py b/readthedocs/core/utils/tasks/retrieve.py index e6bba6e1e..ee1fb362a 100644 --- a/readthedocs/core/utils/tasks/retrieve.py +++ b/readthedocs/core/utils/tasks/retrieve.py @@ -14,9 +14,9 @@ class TaskNotFound(Exception): def get_task_data(task_id): """ Will raise `TaskNotFound` if the task is in state ``PENDING`` or the task + meta data has no ``'task_name'`` key set. """ - result = AsyncResult(task_id) state, info = result.state, result.info if state == 'PENDING': diff --git a/readthedocs/core/views/__init__.py b/readthedocs/core/views/__init__.py index e055e4228..2331849cf 100644 --- a/readthedocs/core/views/__init__.py +++ b/readthedocs/core/views/__init__.py @@ -1,5 +1,6 @@ """ Core views, including the main homepage, + documentation and header rendering, and server errors. """ @@ -37,7 +38,7 @@ class HomepageView(DonateProgressMixin, TemplateView): template_name = 'homepage.html' def get_context_data(self, **kwargs): - '''Add latest builds and featured projects''' + """Add latest builds and featured projects""" context = super(HomepageView, self).get_context_data(**kwargs) latest = [] latest_builds = ( @@ -107,9 +108,7 @@ def divide_by_zero(request): def server_error(request, template_name='500.html'): - """ - A simple 500 handler so we get media - """ + """A simple 500 handler so we get media""" r = render_to_response(template_name, context_instance=RequestContext(request)) r.status_code = 500 @@ -117,9 +116,7 @@ def server_error(request, template_name='500.html'): def server_error_404(request, template_name='404.html'): - """ - A simple 404 handler so we get media - """ + """A simple 404 handler so we get media""" response = get_redirect_response(request, path=request.get_full_path()) if response: return response @@ -132,6 +129,7 @@ def server_error_404(request, template_name='404.html'): class SendEmailView(FormView): """Form view for sending emails to users from admin pages + Accepts the following additional parameters: queryset The queryset to use to determine the users to send emails to @@ -142,6 +140,7 @@ class SendEmailView(FormView): def get_form_kwargs(self): """Override form kwargs based on input fields + The admin posts to this view initially, so detect the send button on form post variables. Drop additional fields if we see the send button. """ @@ -189,6 +188,7 @@ class SendEmailView(FormView): def message_user(self, message, level=messages.INFO, extra_tags='', fail_silently=False): """Implementation of :py:meth:`django.contrib.admin.options.ModelAdmin.message_user` + Send message through messages framework """ # TODO generalize this or check if implementation in ModelAdmin is diff --git a/readthedocs/core/views/hooks.py b/readthedocs/core/views/hooks.py index 4e7439d2c..4fbf356d0 100644 --- a/readthedocs/core/views/hooks.py +++ b/readthedocs/core/views/hooks.py @@ -136,9 +136,7 @@ def _build_url(url, projects, branches): @csrf_exempt def github_build(request): - """ - A post-commit hook for github. - """ + """A post-commit hook for github.""" if request.method == 'POST': try: # GitHub RTD integration @@ -176,9 +174,7 @@ def github_build(request): @csrf_exempt def gitlab_build(request): - """ - A post-commit hook for GitLab. - """ + """A post-commit hook for GitLab.""" if request.method == 'POST': try: # GitLab RTD integration diff --git a/readthedocs/core/views/serve.py b/readthedocs/core/views/serve.py index ee94122cd..00bf6fbc3 100644 --- a/readthedocs/core/views/serve.py +++ b/readthedocs/core/views/serve.py @@ -134,9 +134,7 @@ def _serve_file(request, filename, basepath): @map_subproject_slug def serve_docs(request, project, subproject, lang_slug=None, version_slug=None, filename=''): - """ - This exists mainly to map existing proj, lang, version, filename views to the file format. - """ + """Exists to map existing proj, lang, version, filename views to the file format.""" if not version_slug: version_slug = project.get_default_version() try: