From d537985bb880a8848c0537d9983c19012c7927e7 Mon Sep 17 00:00:00 2001 From: Greg Kaleka Date: Mon, 10 Feb 2025 11:22:17 -0500 Subject: [PATCH] Schedule report tasks and add report preview view (#1620) --- config/celery.py | 9 +- config/urls.py | 6 + core/githubhelper.py | 19 ++- libraries/github.py | 42 +++--- .../management/commands/release_tasks.py | 129 +++++++++++++----- libraries/mixins.py | 10 +- libraries/tasks.py | 10 +- versions/admin.py | 2 +- versions/models.py | 2 +- versions/views.py | 17 +++ 10 files changed, 176 insertions(+), 70 deletions(-) diff --git a/config/celery.py b/config/celery.py index 8c01f932..5895f758 100644 --- a/config/celery.py +++ b/config/celery.py @@ -24,9 +24,10 @@ def debug_task(self): print(f"Request: {self.request!r}") -# Schedule Celery tasks @app.on_after_configure.connect def setup_periodic_tasks(sender, **kwargs): + """Schedule Celery tasks via CeleryBeat.""" + # Update library data from GitHub. Executes daily at 7:05 AM sender.add_periodic_task( crontab(hour=7, minute=5), @@ -56,3 +57,9 @@ def setup_periodic_tasks(sender, **kwargs): datetime.timedelta(minutes=61), app.signature("users.tasks.do_scheduled_user_deletions"), ) + + # Update data required for release report. Executes Saturday evenings. + sender.add_periodic_task( + crontab(day_of_week="sat", hour=20, minute=3), + app.signature("libraries.tasks.release_tasks", generate_report=True), + ) diff --git a/config/urls.py b/config/urls.py index 55a1d882..09f45107 100755 --- a/config/urls.py +++ b/config/urls.py @@ -81,6 +81,7 @@ from versions.views import ( PastReviewListView, ScheduledReviewListView, VersionDetail, + ReportPreviewView, ) djdt_urls = [] @@ -182,6 +183,11 @@ urlpatterns = ( VersionDetail.as_view(), name="release-detail", ), + path( + "releases//report", + ReportPreviewView.as_view(), + name="release-report-preview", + ), path( "donate/", TemplateView.as_view(template_name="donate/donate.html"), diff --git a/core/githubhelper.py b/core/githubhelper.py index 55b90aba..a52feb59 100644 --- a/core/githubhelper.py +++ b/core/githubhelper.py @@ -1,5 +1,4 @@ import base64 -import os import re from collections import defaultdict from datetime import datetime @@ -12,9 +11,14 @@ from zipfile import ZipFile import requests import structlog from dateutil.parser import parse +from django.conf import settings from django.core.exceptions import ValidationError from django.core.validators import validate_email -from fastcore.net import HTTP404NotFoundError, HTTP422UnprocessableEntityError +from fastcore.net import ( + HTTP401UnauthorizedError, + HTTP404NotFoundError, + HTTP422UnprocessableEntityError, +) from fastcore.xtras import obj2dict from ghapi.all import GhApi, paged @@ -38,7 +42,7 @@ class GithubAPIClient: :param ref: str, the Git reference :param repo_slug: str, the repository slug """ - self.token = token or os.environ.get("GITHUB_TOKEN", None) + self.token = token or settings.GITHUB_TOKEN self.api = self.initialize_api() self.owner = owner self.ref = ref @@ -73,6 +77,15 @@ class GithubAPIClient: """ return GhApi(token=self.token) + def is_authenticated(self) -> bool: + if not self.api: + return False + try: + user = self.api.users.get_authenticated() + return bool(user) + except HTTP401UnauthorizedError: + return False + def with_retry(self, fn, retry_count=5): count = 0 while count < 5: diff --git a/libraries/github.py b/libraries/github.py index 899af3d5..89ec604f 100644 --- a/libraries/github.py +++ b/libraries/github.py @@ -66,7 +66,7 @@ class VersionDiffStat: deletions: int -def get_commit_data_for_repo_versions(key): +def get_commit_data_for_repo_versions(key, min_version=""): """Fetch commit data between minor versions (ignore patches). Get commits from one x.x.0 release to the next x.x.0 release. Commits @@ -118,6 +118,9 @@ def get_commit_data_for_repo_versions(key): .values_list("name", flat=True) ) for a, b in zip(versions, versions[1:]): + if a < min_version and b < min_version: + # Don't bother comparing two versions we don't care about + continue shortstat = subprocess.run( ["git", "--git-dir", str(git_dir), "diff", f"{a}..{b}", "--shortstat"], capture_output=True, @@ -348,23 +351,22 @@ class LibraryUpdater: ) if not user: - email = person_data.pop("email") - if not email: - email = generate_fake_email( - f"{person_data['first_name']} {person_data['last_name']}" - ) - user = User.objects.create_stub_user(email.lower(), **person_data) - self.logger.info(f"User {user.email} created.") + email = person_data.pop("email") or generate_fake_email( + f"{person_data['first_name']} {person_data['last_name']}" + ) + if not (user := User.objects.filter(email=email).first()): + user = User.objects.create_stub_user(email.lower(), **person_data) + self.logger.info(f"User {user.email} created.") obj.maintainers.add(user) self.logger.info(f"User {user.email} added as a maintainer of {obj}") - def update_issues(self, obj): + def update_issues(self, library): """Import GitHub issues for the library and update the database""" self.logger.info("updating_repo_issues") issues_data = self.client.get_repo_issues( - self.client.owner, obj.github_repo, state="all", issues_only=True + self.client.owner, library.github_repo, state="all", issues_only=True ) for issue_dict in issues_data: # Get the date information @@ -384,7 +386,7 @@ class LibraryUpdater: # Create or update the Issue object try: issue, created = Issue.objects.update_or_create( - library=obj, + library=library, github_id=issue_dict["id"], defaults={ "title": issue_dict["title"][:255], @@ -410,11 +412,11 @@ class LibraryUpdater: ) continue - def update_prs(self, obj): + def update_prs(self, library: Library): """Update all PRs for a library""" self.logger.info("updating_repo_prs") - prs_data = self.client.get_repo_prs(obj.github_repo, state="all") + prs_data = self.client.get_repo_prs(library.github_repo, state="all") for pr_dict in prs_data: # Get the date information @@ -437,7 +439,7 @@ class LibraryUpdater: try: pull_request, created = PullRequest.objects.update_or_create( - library=obj, + library=library, github_id=pr_dict["id"], defaults={ "title": pr_dict["title"][:255], @@ -463,15 +465,15 @@ class LibraryUpdater: exc_msg=str(e), ) - def update_commits(self, obj: Library, clean=False): + def update_commits(self, library: Library, clean=False, min_version=""): """Import a record of all commits between LibraryVersions.""" authors = {} commits = [] library_versions = { x.version.name: x - for x in LibraryVersion.objects.filter(library=obj).select_related( - "version" - ) + for x in LibraryVersion.objects.filter( + library=library, version__name__gte=min_version + ).select_related("version") } library_version_updates = [] @@ -509,7 +511,7 @@ class LibraryUpdater: return lv commits_handled = 0 - for item in get_commit_data_for_repo_versions(obj.key): + for item in get_commit_data_for_repo_versions(library.key, min_version): match item: case ParsedCommit(): commits_handled += 1 @@ -521,7 +523,7 @@ class LibraryUpdater: with transaction.atomic(): if clean: - Commit.objects.filter(library_version__library=obj).delete() + Commit.objects.filter(library_version__library=library).delete() Commit.objects.bulk_create( commits, update_conflicts=True, diff --git a/libraries/management/commands/release_tasks.py b/libraries/management/commands/release_tasks.py index d54906e1..262f7644 100644 --- a/libraries/management/commands/release_tasks.py +++ b/libraries/management/commands/release_tasks.py @@ -1,4 +1,5 @@ import traceback +from contextlib import suppress import djclick as click @@ -7,18 +8,21 @@ from django.utils import timezone from django.core.management import call_command from django.contrib.auth import get_user_model from django.conf import settings +from slack_sdk.errors import SlackApiError +from core.githubhelper import GithubAPIClient +from libraries.forms import CreateReportForm from libraries.tasks import update_commits -from slack.management.commands.fetch_slack_activity import locked - +from slack.management.commands.fetch_slack_activity import get_my_channels, locked +from versions.models import Version User = get_user_model() -def send_notification(user, message): +def send_notification(user, message, subject="Task Started: release_tasks"): if user.email: send_mail( - "Task Started: release_tasks", + subject, message, settings.DEFAULT_FROM_EMAIL, [user.email], @@ -31,24 +35,27 @@ def progress_message(message: str): @locked(1138692) -def run_commands(progress: list[str]): +def run_commands(progress: list[str], generate_report: bool = False): if not settings.SLACK_BOT_TOKEN: raise ValueError("SLACK_BOT_TOKEN is not set.") handled_commits = {} progress.append(progress_message("Importing versions...")) call_command("import_versions", "--new") progress.append(progress_message("Finished importing versions.")) + latest_version: Version = Version.objects.most_recent() + latest_version_name = latest_version.name progress.append(progress_message("Importing most recent beta version...")) call_command("import_beta_release", "--delete-versions") progress.append(progress_message("Finished importing most recent beta version.")) - progress.append(progress_message("Importing libraries")) + progress.append(progress_message("Importing libraries...")) call_command("update_libraries") progress.append(progress_message("Finished importing libraries.")) progress.append(progress_message("Saving library-version relationships...")) - call_command("import_library_versions") + latest_version_number = latest_version_name.lstrip("boost-") + call_command("import_library_versions", min_release=latest_version_number) progress.append(progress_message("Finished saving library-version relationships.")) progress.append(progress_message("Adding library maintainers...")) @@ -64,12 +71,12 @@ def run_commands(progress: list[str]): progress.append(progress_message("Finished adding library version authors.")) progress.append(progress_message("Importing git commits...")) - handled_commits = update_commits() - progress.append(progress_message("Finished importing commits...")) + handled_commits = update_commits(min_version=latest_version_name) + progress.append(progress_message("Finished importing commits.")) progress.append(progress_message("Syncing mailinglist statistics...")) call_command("sync_mailinglist_stats") - progress.append(progress_message("Finished syncing mailinglist statistics...")) + progress.append(progress_message("Finished syncing mailinglist statistics.")) progress.append(progress_message("Updating github issues...")) call_command("update_issues") @@ -77,11 +84,46 @@ def run_commands(progress: list[str]): progress.append(progress_message("Updating slack activity buckets...")) call_command("fetch_slack_activity") - progress.append(progress_message("Finished updating slack activity buckets...")) + progress.append(progress_message("Finished updating slack activity buckets.")) + + if generate_report: + progress.append( + progress_message(f"Generating report for {latest_version_name}...") + ) + form = CreateReportForm({"version": latest_version.id}) + form.cache_html() + progress.append( + progress_message(f"Finished generating report for {latest_version_name}.") + ) return handled_commits +def bad_credentials() -> list[str]: + """This management command requires access to Slack and GitHub APIs. + Checks that credentials are available and valid. + + Returns a list of credentials that are invalid or missing. + + Good return: [] + Bad return: ["GITHUB_TOKEN", "SLACK_BOT_TOKEN"] + """ + possibly_bad_credentials = ["GITHUB_TOKEN", "SLACK_BOT_TOKEN"] + if settings.GITHUB_TOKEN: + client = GithubAPIClient(settings.GITHUB_TOKEN) + if client.is_authenticated(): + # If this is true, the GitHub token is good + possibly_bad_credentials.remove("GITHUB_TOKEN") + + if settings.SLACK_BOT_TOKEN: + with suppress(SlackApiError): # just breaks on this error + next(get_my_channels()) + # If we get this far, the Slack token is good + possibly_bad_credentials.remove("SLACK_BOT_TOKEN") + + return possibly_bad_credentials + + @click.command() @click.option( "--user_id", @@ -89,7 +131,13 @@ def run_commands(progress: list[str]): help="The ID of the user that started this task (For notification purposes)", default=None, ) -def command(user_id=None): +@click.option( + "--generate_report", + is_flag=True, + help="Generate a report at the end of the command", + default=False, +) +def command(user_id=None, generate_report=False): """A long running chain of tasks to import and update library data.""" start = timezone.now() @@ -97,12 +145,23 @@ def command(user_id=None): if user_id: user = User.objects.filter(id=user_id).first() + progress = ["___Progress Messages___"] + if missing_creds := bad_credentials(): + progress.append( + progress_message(f"Missing credentials {', '.join(missing_creds)}") + ) + if user: + send_notification( + user, + message="Your task `release_tasks` failed.", + subject="Task Failed: release_tasks", + ) + return if user: send_notification(user, f"Your task `release_tasks` was started at: {start}") - progress = ["___Progress Messages___"] - handled_commits = {} + try: - handled_commits = run_commands(progress) + handled_commits = run_commands(progress, generate_report) end = timezone.now() progress.append(progress_message(f"All done! Completed in {end - start}")) except Exception: @@ -117,24 +176,24 @@ def command(user_id=None): "\n\n".join(message), ) raise - else: - zero_commit_libraries = [ - (key, val) for key, val in handled_commits.items() if val == 0 + + zero_commit_libraries = [ + (key, val) for key, val in handled_commits.items() if val == 0 + ] + message = [ + f"The task `release_tasks` was completed. Task took: {end - start}", + "\n".join(progress), + ] + if zero_commit_libraries: + zero_commit_message = [ + "The import_commits task did not find commits for these libraries.", + "The task may need to re-run.", ] - message = [ - f"The task `release_tasks` was completed. Task took: {end - start}", - "\n".join(progress), - ] - if zero_commit_libraries: - zero_commit_message = [ - "The import_commits task did not find commits for these libraries.", - "The task may need to re-run.", - ] - for lib, _ in zero_commit_libraries: - zero_commit_message.append(lib) - message.append("\n".join(zero_commit_message)) - if user: - send_notification( - user, - "\n\n".join(message), - ) + for lib, _ in zero_commit_libraries: + zero_commit_message.append(lib) + message.append("\n".join(zero_commit_message)) + if user: + send_notification( + user, + "\n\n".join(message), + ) diff --git a/libraries/mixins.py b/libraries/mixins.py index 0f1b5bd9..accf5b31 100644 --- a/libraries/mixins.py +++ b/libraries/mixins.py @@ -32,12 +32,14 @@ class VersionAlertMixin: class BoostVersionMixin: def dispatch(self, request, *args, **kwargs): + self.set_extra_context(request) + return super().dispatch(request, *args, **kwargs) + + def set_extra_context(self, request): if not self.extra_context: self.extra_context = {} - if not self.extra_context.get("current_version"): self.extra_context["current_version"] = Version.objects.most_recent() - self.extra_context.update( { "version_str": self.kwargs.get("version_slug"), @@ -52,7 +54,6 @@ class BoostVersionMixin: self.extra_context["selected_version"] = get_object_or_404( Version, slug=self.extra_context["version_str"] ) - version_path_kwargs = {} # Only when the user uses master or develop do those versions to appear if self.extra_context["version_str"] in [ @@ -60,15 +61,12 @@ class BoostVersionMixin: DEVELOP_RELEASE_URL_PATH_STR, ]: version_path_kwargs[f"allow_{self.extra_context['version_str']}"] = True - if self.request.resolver_match.view_name == "library-detail": version_path_kwargs["flag_versions_without_library"] = get_object_or_404( Library, slug=self.kwargs.get("library_slug") ) - self.extra_context["versions"] = Version.objects.get_dropdown_versions( **version_path_kwargs ) # here we hack extra_context into the request so we can access for cookie checks request.extra_context = self.extra_context - return super().dispatch(request, *args, **kwargs) diff --git a/libraries/tasks.py b/libraries/tasks.py index 26469c3c..b283e648 100644 --- a/libraries/tasks.py +++ b/libraries/tasks.py @@ -192,13 +192,15 @@ def update_authors_and_maintainers(): @app.task -def update_commits(token=None, clean=False): +def update_commits(token=None, clean=False, min_version=""): # dictionary of library_key: int commits_handled: dict[str, int] = {} updater = LibraryUpdater(token=token) for library in Library.objects.all(): logger.info("Importing commits for library.", library=library) - commits_handled[library.key] = updater.update_commits(obj=library, clean=clean) + commits_handled[library.key] = updater.update_commits( + library=library, clean=clean, min_version=min_version + ) logger.info("update_commits finished.") return commits_handled @@ -241,7 +243,7 @@ def update_library_version_dependencies(token=None): @app.task -def release_tasks(user_id=None): +def release_tasks(user_id=None, generate_report=False): """Call the release_tasks management command. If a user_id is given, that user will receive an email at the beginning @@ -251,4 +253,6 @@ def release_tasks(user_id=None): command = ["release_tasks"] if user_id: command.extend(["--user_id", user_id]) + if generate_report: + command.append("--generate_report") call_command(*command) diff --git a/versions/admin.py b/versions/admin.py index 9b24ef05..b7310cc6 100755 --- a/versions/admin.py +++ b/versions/admin.py @@ -50,7 +50,7 @@ class VersionAdmin(admin.ModelAdmin): return my_urls + urls def release_tasks(self, request): - release_tasks.delay(user_id=request.user.id) + release_tasks.delay(user_id=request.user.id, generate_report=True) self.message_user( request, "release_tasks has started, you will receive an email when the task finishes.", # noqa: E501 diff --git a/versions/models.py b/versions/models.py index 393f1eba..ede640a5 100755 --- a/versions/models.py +++ b/versions/models.py @@ -205,7 +205,7 @@ class Version(models.Model): @cached_property def release_notes_cache_key(self): - """Returns the cahe key used to access the release notes in the + """Returns the cache key used to access the release notes in the RenderedContent model.""" version = "-".join(self.cleaned_version_parts) return f"release_notes_boost-{version}" diff --git a/versions/views.py b/versions/views.py index 4d4ffd1a..b8485119 100755 --- a/versions/views.py +++ b/versions/views.py @@ -1,8 +1,11 @@ +from django.contrib.admin.views.decorators import staff_member_required from django.db.models.query import QuerySet from itertools import groupby from operator import attrgetter from django.db.models import Q, Count +from django.http import HttpResponse +from django.views import View from django.views.generic import DetailView, TemplateView, ListView from django.shortcuts import redirect, get_object_or_404 from django.contrib import messages @@ -175,3 +178,17 @@ class ScheduledReviewListView(ListView): def get_queryset(self) -> QuerySet[Review]: qs = super().get_queryset() return qs.exclude(results__isnull=False).distinct() + + +@method_decorator(staff_member_required, name="get") +class ReportPreviewView(BoostVersionMixin, View): + extra_context = {} + + def get(self, request, *args, **kwargs): + version_name = self.extra_context["selected_version"].name + # TODO: this is a bit silly. There's probably a more elegant solution + cache_key = f"release-report-,,,,,,,-{version_name}" + # TODO: it might be better to show a friendly "report not yet generated" + # message instead of 404ing. + content = get_object_or_404(RenderedContent, cache_key=cache_key) + return HttpResponse(content.content_html)