Improvements based on proposed changes for tag cleanup

- Make importing tags for a project an admin function
- Use the github app Oauth credentials
- Make removing unused tags a reusable function
  so it can be reused in the future by celery

Author: davidfischer

readthedocs/core/management/commands/import_github_tags.py

@@ -1,74 +1,28 @@
-"""
-Import a project's tags from GitHub.
-
-Requires a ``GITHUB_AUTH_TOKEN`` to be set in the environment.
-This should be a "Personal access token" although no permissions are required.
-With the token, the rate limit is increased to 5,000 requests/hour
-
-https://github.com/settings/tokens
-https://developer.github.com/v3/#rate-limiting
-"""
-
-import os
+"""Imports tags for projects without tags from GitHub."""
 import time
 
-import requests
-from django.core.management.base import BaseCommand, CommandError
+from django.core.management.base import BaseCommand
 
-from readthedocs.projects.constants import GITHUB_REGEXS
 from readthedocs.projects.models import Project
+from readthedocs.projects.tag_utils import import_tags
 
 
 class Command(BaseCommand):
 
     help = __doc__
 
     def handle(self, *args, **options):
-        token = os.environ.get('GITHUB_AUTH_TOKEN')
-        if not token:
-            raise CommandError('Invalid GitHub token, exiting...')
-
         queryset = Project.objects.filter(tags=None).filter(repo__contains='github.com')
         project_total = queryset.count()
 
         for i, project in enumerate(queryset.iterator()):
-            # Get the user and repo name from the URL as required by GitHub's API
-            user = repo = ''
-            for regex in GITHUB_REGEXS:
-                match = regex.search(project.repo)
-                if match:
-                    user, repo = match.groups()
-                    break
-
-            if not user:
-                self.stderr.write(f'No GitHub repo for {project.repo}')
-                continue
-
-            # https://developer.github.com/v3/repos/#list-all-topics-for-a-repository
-            url = 'https://api.github.com/repos/{user}/{repo}/topics'.format(
-                user=user,
-                repo=repo,
-            )
-            headers = {
-                'Authorization': 'token {token}'.format(token=token),
-
-                # Getting topics is a preview API and may change
-                # It requires this custom Accept header
-                'Accept': 'application/vnd.github.mercy-preview+json',
-            }
-
             self.stdout.write(
                 '[{}/{}] Fetching tags for {}'.format(i + 1, project_total, project.slug)
             )
 
-            resp = requests.get(url, headers=headers)
-            if resp.ok:
-                tags = resp.json()['names']
-                if tags:
-                    self.stdout.write('Setting tags for {}: {}'.format(project.slug, tags))
-                    project.tags.set(*tags)
-            else:
-                self.stderr.write('GitHub API error ({}): {}'.format(project.slug, resp.content))
+            tags = import_tags(project)
+            if tags:
+                self.stdout.write('Set tags for {}: {}'.format(project.slug, tags))
 
             # Sleeping half a second should keep us under 5k requests/hour
             time.sleep(0.5)

readthedocs/core/tag_utils.py

@@ -29,6 +29,7 @@
     ResourceUsageNotification,
 )
 from .tasks import remove_dirs
+from .tag_utils import import_tags
 
 
 class ProjectSendNotificationView(SendNotificationView):
@@ -155,6 +156,7 @@ class ProjectAdmin(admin.ModelAdmin):
         'build_default_version',
         'reindex_active_versions',
         'wipe_all_versions',
+        'import_tags_from_vcs',
     ]
 
     def feature_flags(self, obj):
@@ -294,6 +296,24 @@ def wipe_all_versions(self, request, queryset):
 
     wipe_all_versions.short_description = 'Wipe all versions from ES'
 
+    def import_tags_from_vcs(self, request, queryset):
+        for project in queryset.iterator():
+            tags = import_tags(project)
+            if tags:
+                self.message_user(
+                    request,
+                    'Imported tags for {}: {}'.format(project, tags),
+                    messages.SUCCESS
+                )
+            else:
+                self.message_user(
+                    request,
+                    'No tags found for {}'.format(project),
+                    messages.WARNING
+                )
+
+    import_tags_from_vcs.short_description = 'Import tags from the version control API'
+
     def get_actions(self, request):
         actions = super().get_actions(request)
         actions['delete_selected'] = (

readthedocs/projects/admin.py

@@ -6,14 +6,17 @@
 - Differences only in lowercase/uppercase
 - Slugify all tags
 - Remove tags with no projects (old & spam mostly)
+
+This command can probably be completely removed after being run.
+Future tags should be canonicalized because of the new tag parser in
+``readthedocs.projects.tag_utils.rtd_parse_tags``
 """
 
 from django.core.management.base import BaseCommand
-from django.db.models import Count
-from taggit.models import Tag
 from taggit.utils import parse_tags, edit_string_for_tags
 
 from readthedocs.projects.models import Project
+from readthedocs.projects.tag_utils import remove_unused_tags
 
 
 class Command(BaseCommand):
@@ -57,23 +60,27 @@ def reprocess_tags(self):
                         )
 
     def remove_tags_with_no_projects(self):
-        self.stdout.write('Removing tags with no projects...')
-        for tag in Tag.objects.all().annotate(num=Count('taggit_taggeditem_items')).filter(num=0):
-            if not self.dry_run:
-                self.stdout.write('Removing tag {}'.format(tag.name))
-                tag.delete()
-            else:
-                self.stdout.write('Not removing tag "{}" (dry run)'.format(tag.name))
+        if not self.dry_run:
+            self.stdout.write('Removing tags with no projects...')
+            num_deleted, _ = remove_unused_tags()
+            self.stdout.write('{} unused tags deleted'.format(num_deleted))
 
     def add_arguments(self, parser):
         parser.add_argument(
             "--dry-run",
             action="store_true",
             help="Don't actually perform the actions, just print output",
         )
+        parser.add_argument(
+            "--remove-unused-only",
+            action="store_true",
+            help="Don't canonicalize tags, just delete unused",
+        )
 
     def handle(self, *args, **options):
         self.dry_run = options["dry_run"]
 
-        self.reprocess_tags()
+        if not options["remove_unused_only"]:
+            self.reprocess_tags()
+
         self.remove_tags_with_no_projects()

readthedocs/core/management/commands/clean_tags.py renamed to readthedocs/projects/management/commands/clean_tags.py

@@ -0,0 +1,84 @@
+"""Customizations to Django Taggit."""
+from allauth.socialaccount.models import SocialApp
+from django.db.models import Count
+from django.utils.text import slugify
+import requests
+from taggit.models import Tag
+from taggit.utils import _parse_tags
+
+from .constants import GITHUB_REGEXS
+
+
+def rtd_parse_tags(tag_string):
+    """
+    Parses a string into its tags.
+
+    - Lowercases all tags
+    - Converts underscores to hyphens
+    - Slugifies tags
+
+    :see: https://django-taggit.readthedocs.io/page/custom_tagging.html
+    :param tag_string: a delimited string of tags
+    :return: a sorted list of tag strings
+    """
+    if tag_string:
+        tag_string = tag_string.lower().replace('_', '-')
+
+    return sorted([slugify(tag) for tag in _parse_tags(tag_string)])
+
+
+def remove_unused_tags():
+    """Removes all tags that have no corresponding items (projects)."""
+    return Tag.objects.all().annotate(
+        num=Count('taggit_taggeditem_items')
+    ).filter(num=0).delete()
+
+
+def import_tags(project):
+    """
+    Import tags using the version control API.
+
+    Currently, this is only implemented for github.
+    Uses the client ID and client secret for github otherwise the rate limit is 60/hr.
+    https://developer.github.com/v3/#rate-limiting
+
+    :returns: A list of the tags set or ``None`` on an error
+    """
+    user = repo = ''
+    for regex in GITHUB_REGEXS:
+        match = regex.search(project.repo)
+        if match:
+            user, repo = match.groups()
+            break
+
+    if not user:
+        return None
+
+    provider = SocialApp.objects.filter(provider='github').first()
+    if not provider:
+        return None
+
+    # https://developer.github.com/v3/repos/#list-all-topics-for-a-repository
+    url = 'https://api.github.com/repos/{user}/{repo}/topics'.format(
+        user=user,
+        repo=repo,
+    )
+    headers = {
+        # Getting topics is a preview API and may change
+        # It requires this custom Accept header
+        'Accept': 'application/vnd.github.mercy-preview+json',
+    }
+    params = {
+        'client_id': provider.client_id,
+        'client_secret': provider.secret,
+    }
+
+    resp = requests.get(url, headers=headers, params=params)
+    if resp.ok:
+        tags = resp.json()['names']
+        if tags:
+            project.tags.set(*tags)
+            return tags
+        return []
+
+    return None

readthedocs/projects/tag_utils.py

@@ -465,7 +465,7 @@ def USE_PROMOS(self):  # noqa
 
     # Taggit
     # https://django-taggit.readthedocs.io
-    TAGGIT_TAGS_FROM_STRING = 'readthedocs.core.tag_utils.rtd_parse_tags'
+    TAGGIT_TAGS_FROM_STRING = 'readthedocs.projects.tag_utils.rtd_parse_tags'
 
     # Stripe
     STRIPE_SECRET = None