From 7921bf3f515e538eb2f87b6fc3a054e46559bc6b Mon Sep 17 00:00:00 2001 From: daveoconnor Date: Wed, 28 Jan 2026 12:38:23 -0800 Subject: [PATCH] fully_imported check on Version manager (#1995) (#2024) --- libraries/github.py | 1 - .../import_library_version_docs_urls.py | 11 +- .../commands/import_library_versions.py | 10 +- .../management/commands/release_tasks.py | 3 +- libraries/tasks.py | 4 +- libraries/tests/test_models.py | 4 +- .../commands/sync_mailinglist_stats.py | 8 +- versions/admin.py | 7 +- .../commands/import_archives_release_data.py | 6 +- versions/managers.py | 6 +- versions/releases.py | 10 +- versions/tasks.py | 55 +++-- versions/tests/test_managers.py | 228 ++++++++++++++++++ 13 files changed, 305 insertions(+), 48 deletions(-) diff --git a/libraries/github.py b/libraries/github.py index 7a855ef7..9f1b7912 100644 --- a/libraries/github.py +++ b/libraries/github.py @@ -506,7 +506,6 @@ class LibraryUpdater: ) except KeyError: - logger.warning(f"KeyError {commit.version=}") return None def handle_version_diff_stat(diff: VersionDiffStat): diff --git a/libraries/management/commands/import_library_version_docs_urls.py b/libraries/management/commands/import_library_version_docs_urls.py index 7321a760..fe8c9b44 100644 --- a/libraries/management/commands/import_library_version_docs_urls.py +++ b/libraries/management/commands/import_library_version_docs_urls.py @@ -47,12 +47,15 @@ def command(release: str, new: bool, min_version: str): processed. """ click.secho("Saving links to version-specific library docs...", fg="green") - min_version = f"boost-{min_version}" - version_qs = Version.objects.active().filter(name__gte=min_version) + version_qs = ( + Version.objects.with_partials() + .active() + .filter(name__gte=f"boost-{min_version}") + ) if release: - versions = Version.objects.filter(name__icontains=release).order_by("-name") + versions = version_qs.filter(name__icontains=release).order_by("-name") elif new: - versions = [Version.objects.most_recent()] + versions = [version_qs.most_recent()] else: versions = version_qs.order_by("-name") diff --git a/libraries/management/commands/import_library_versions.py b/libraries/management/commands/import_library_versions.py index 3987ceaf..e809b8fc 100644 --- a/libraries/management/commands/import_library_versions.py +++ b/libraries/management/commands/import_library_versions.py @@ -43,13 +43,15 @@ def command(min_release: str, release: str, new: bool, token: str): Overridden by --release if provided. """ click.secho("Saving library-version relationships...", fg="green") - - min_release = f"boost-{min_release}" - versions_qs = Version.objects.active().filter(name__gte=min_release) + versions_qs = ( + Version.objects.with_partials() + .active() + .filter(name__gte=f"boost-{min_release}") + ) if release: versions = versions_qs.filter(name__icontains=release).order_by("-name") elif new: - versions = [Version.objects.most_recent()] + versions = [versions_qs.most_recent()] else: versions = versions_qs.order_by("-name") diff --git a/libraries/management/commands/release_tasks.py b/libraries/management/commands/release_tasks.py index 58905fe5..d896f917 100644 --- a/libraries/management/commands/release_tasks.py +++ b/libraries/management/commands/release_tasks.py @@ -44,6 +44,7 @@ class ReleaseTasksManager(ActionsManager): "Importing most recent beta version", ["import_beta_release", "--delete-versions"], ), + Action("Importing development versions", ["import_development_versions"]), Action("Importing libraries", ["update_libraries"]), Action( "Saving library-version relationships", self.import_library_versions @@ -64,7 +65,7 @@ class ReleaseTasksManager(ActionsManager): def import_versions(self): call_command("import_versions") - self.latest_version = Version.objects.most_recent() + self.latest_version = Version.objects.with_partials().most_recent() def import_library_versions(self): latest_version_number = self.latest_version.name.lstrip("boost-") diff --git a/libraries/tasks.py b/libraries/tasks.py index d3a10901..ad0a0afc 100644 --- a/libraries/tasks.py +++ b/libraries/tasks.py @@ -47,7 +47,7 @@ logger = structlog.getLogger(__name__) @app.task def update_library_version_documentation_urls_all_versions(): """Run the task to update all documentation URLs for all versions""" - for version in Version.objects.all().order_by("-name"): + for version in Version.objects.with_partials().all().order_by("-name"): get_and_store_library_version_documentation_urls_for_version(version.pk) @@ -68,7 +68,7 @@ def get_and_store_library_version_documentation_urls_for_version(version_pk): database. """ try: - version = Version.objects.get(pk=version_pk) + version = Version.objects.with_partials().get(pk=version_pk) except Version.DoesNotExist: logger.error(f"Version does not exist for {version_pk=}") raise diff --git a/libraries/tests/test_models.py b/libraries/tests/test_models.py index a845a15c..441e6593 100644 --- a/libraries/tests/test_models.py +++ b/libraries/tests/test_models.py @@ -77,7 +77,9 @@ def test_library_version_multiple_versions(library, library_version): assert library.versions.filter( library_version__version=library_version.version ).exists() - other_version = baker.make("versions.Version", name="New Version") + other_version = baker.make( + "versions.Version", name="New Version", fully_imported=True + ) baker.make("libraries.LibraryVersion", library=library, version=other_version) assert library.versions.count() == 2 assert library.versions.filter( diff --git a/mailing_list/management/commands/sync_mailinglist_stats.py b/mailing_list/management/commands/sync_mailinglist_stats.py index 7da563eb..2b73be99 100644 --- a/mailing_list/management/commands/sync_mailinglist_stats.py +++ b/mailing_list/management/commands/sync_mailinglist_stats.py @@ -58,12 +58,16 @@ def create_emaildata(conn: Connection): versions = Version.objects.minor_versions().order_by("version_array") columns = ["email", "name", "count"] - versions = list(versions) + [Version.objects.get(name="master")] + versions = list(versions) + [Version.objects.with_partials().get(name="master")] for a, b in pairwise(versions): start = a.release_date end = b.release_date or date.today() if not (start and end): - raise ValueError("All x.x.0 versions must have a release date.") + msg = ( + "All x.x.0 versions must have a release date." + f" {a=} {b=} {a.release_date=} {b.release_date=}" + ) + raise ValueError(msg) with conn.cursor(name=f"emaildata_sync_{b.name}") as cursor: cursor.execute( """ diff --git a/versions/admin.py b/versions/admin.py index b79a0e87..5eba1a92 100755 --- a/versions/admin.py +++ b/versions/admin.py @@ -7,6 +7,7 @@ from django.urls import path from libraries.tasks import import_new_versions_tasks from . import models +from .models import Version class VersionFileInline(admin.StackedInline): @@ -23,9 +24,9 @@ class VersionAdmin(admin.ModelAdmin): "name", "release_date", "active", - "full_release", "beta", "fully_imported", + "full_release", ] list_filter = ["active", "full_release", "beta"] ordering = ["-release_date", "-name"] @@ -34,6 +35,10 @@ class VersionAdmin(admin.ModelAdmin): inlines = [VersionFileInline] change_list_template = "admin/version_change_list.html" + def get_queryset(self, request: HttpRequest) -> QuerySet: + # we want all versions here, including not fully_imported + return Version.objects.with_partials() + def get_urls(self): urls = super().get_urls() my_urls = [ diff --git a/versions/management/commands/import_archives_release_data.py b/versions/management/commands/import_archives_release_data.py index 87bbfe8d..7b256fec 100644 --- a/versions/management/commands/import_archives_release_data.py +++ b/versions/management/commands/import_archives_release_data.py @@ -44,11 +44,11 @@ def command(release: str, new: bool): if release: name = f"boost-{release}" if release not in ["master", "develop"] else release - versions = [Version.objects.get(name=name)] + versions = [Version.objects.with_partials().get(name=name)] elif new: - versions = [Version.objects.most_recent()] + versions = [Version.objects.with_partials().most_recent()] else: - versions = Version.objects.filter(name__gte=last_release) + versions = Version.objects.with_partials().filter(name__gte=last_release) logger.info(f"import_archive_release_data {versions=}") for v in versions: diff --git a/versions/managers.py b/versions/managers.py index d1222aa5..0b6363ce 100644 --- a/versions/managers.py +++ b/versions/managers.py @@ -12,7 +12,7 @@ from libraries.constants import ( class VersionQuerySet(models.QuerySet): def active(self): """Return active versions""" - return self.filter(active=True, fully_imported=True) + return self.filter(active=True) def most_recent(self): """Return most recent active non-beta version""" @@ -61,6 +61,10 @@ class VersionQuerySet(models.QuerySet): class VersionManager(models.Manager): def get_queryset(self): + return VersionQuerySet(self.model, using=self._db).filter(fully_imported=True) + + def with_partials(self): + """Return all objects regardless of fully_imported status""" return VersionQuerySet(self.model, using=self._db) def active(self): diff --git a/versions/releases.py b/versions/releases.py index 185926e8..8b431e24 100644 --- a/versions/releases.py +++ b/versions/releases.py @@ -36,9 +36,7 @@ def get_download_uris_for_release( resp = session.get(release_path) resp.raise_for_status() except requests.exceptions.HTTPError as e: - logger.error( - "get_archives_releases_list_error", exc_msg=str(e), url=release_path - ) + logger.error(f"get_archives_releases_list_error {str(e)=}, {release_path=}") raise soup = BeautifulSoup(resp.text, "html.parser") @@ -207,7 +205,7 @@ def get_release_notes_for_version_s3(version_pk): # and are not extensible if we encounter additional filename patterns in the # future; we should refactor. try: - version = Version.objects.get(pk=version_pk) + version = Version.objects.with_partials().get(pk=version_pk) except Version.DoesNotExist: logger.info( "get_release_notes_for_version_s3_error_version_not_found", @@ -244,7 +242,7 @@ def get_release_notes_for_version_github(version_pk): # and are not extensible if we encounter additional filename patterns in the # future; we should refactor. try: - version = Version.objects.get(pk=version_pk) + version = Version.objects.with_partials().get(pk=version_pk) except Version.DoesNotExist: logger.info( "get_release_notes_for_version_error_version_not_found", @@ -326,7 +324,7 @@ def store_release_notes_for_version(version_pk): # Get the version # todo: convert to task, remove the task that calls this, is redundant try: - version = Version.objects.get(pk=version_pk) + version = Version.objects.with_partials().get(pk=version_pk) except Version.DoesNotExist: logger.info(f"store_release_notes version_not_found {version_pk=}") raise Version.DoesNotExist diff --git a/versions/tasks.py b/versions/tasks.py index 59943f12..7ed73c8f 100644 --- a/versions/tasks.py +++ b/versions/tasks.py @@ -2,7 +2,7 @@ from django.db import transaction import requests import structlog -from celery import group +from celery import group, chain from config.celery import app from django.conf import settings @@ -47,11 +47,11 @@ def import_versions( imports are finished. """ if delete_versions: - Version.objects.all().delete() + Version.objects.with_partials().all().delete() logger.info("import_versions_deleted_all_versions") # delete any versions that were only partially imported so they are re-imported - Version.objects.filter(fully_imported=False).delete() + Version.objects.with_partials().filter(fully_imported=False).delete() # Get all Boost tags from Github client = GithubAPIClient(token=token) @@ -82,7 +82,7 @@ def import_versions( def import_release_notes(new_versions_only=True): """Imports release notes from the existing rendered release notes in the repository.""" - versions = [Version.objects.most_recent()] + versions = [Version.objects.with_partials().most_recent()] if not new_versions_only: versions = ( Version.objects.exclude(name__in=["master", "develop"]) @@ -101,7 +101,7 @@ def import_release_notes(new_versions_only=True): def store_release_notes_task(version_pk): """Stores the release notes for a single version.""" try: - Version.objects.get(pk=version_pk) + Version.objects.with_partials().get(pk=version_pk) except Version.DoesNotExist: logger.error(f"store_release_notes_task_version_does_not_exist {version_pk=}") return @@ -137,7 +137,7 @@ def import_version( else: data = {} - version, created = Version.objects.update_or_create( + version, created = Version.objects.with_partials().update_or_create( name=name, defaults={ "github_url": f"{base_url}{name}", @@ -166,17 +166,30 @@ def import_development_versions(): """Imports the `master` and `develop` branches as Versions""" base_url = "https://github.com/boostorg/boost/tree/" + import_version_tasks = [] + import_library_version_tasks = [] for branch in settings.BOOST_BRANCHES: - import_version.delay( - branch, - branch, - beta=False, - full_release=False, - get_release_date=False, - base_url=base_url, + import_version_tasks.append( + import_version.s( + branch, + branch, + beta=False, + full_release=False, + get_release_date=False, + base_url=base_url, + ) ) - import_library_versions.delay(branch, version_type="branch") + import_library_version_tasks.append( + import_library_versions.s(branch, version_type="branch") + ) + + task_chain = chain( + group(*import_version_tasks), + group(*import_library_version_tasks), + mark_fully_completed.s(), + ) + task_chain() @app.task @@ -269,7 +282,7 @@ def import_library_versions(version_name, token=None, version_type="tag"): """For a specific version, imports all LibraryVersions using GitHub data""" # todo: this needs to be refactored and tests added try: - version = Version.objects.get(name=version_name) + version = Version.objects.with_partials().get(name=version_name) except Version.DoesNotExist: logger.info( "import_library_versions_version_not_found", version_name=version_name @@ -406,7 +419,7 @@ def import_library_versions(version_name, token=None, version_type="tag"): @app.task def import_release_downloads(version_pk): logger.info(f"import_release_downloads w/ {version_pk=}") - version = Version.objects.get(pk=version_pk) + version = Version.objects.with_partials().get(pk=version_pk) version_num = version.name.replace("boost-", "") if version_num < "1.63.0": # Downloads are in Sourceforge for older versions, and that has @@ -427,11 +440,9 @@ def get_release_date_for_version(version_pk, commit_sha, token=None): :param commit_sha: The SHA of the commit to get the release date for. """ try: - version = Version.objects.get(pk=version_pk) + version = Version.objects.with_partials().get(pk=version_pk) except Version.DoesNotExist: - logger.error( - "get_release_date_for_version_no_version_found", version_pk=version_pk - ) + logger.error(f"get_release_date_for_version_no_version_found {version_pk=}") return if not token: @@ -490,7 +501,7 @@ def purge_fastly_release_cache(): @app.task def mark_fully_completed(beta_only=False, full_release_only=False): """Marks all versions as fully imported""" - qs = Version.objects.filter(fully_imported=False) + qs = Version.objects.with_partials().filter(fully_imported=False) if full_release_only: logger.info("Marking active as fully imported") qs = qs.filter(full_release=True) @@ -523,7 +534,7 @@ def skip_tag(name, new=False): EXCLUSIONS = ["beta", "-rc", "-bgl"] # If we are only importing new versions, and we already have this one, skip - if new and Version.objects.filter(name=name).exists(): + if new and Version.objects.with_partials().filter(name=name).exists(): return True # If this version falls in our exclusion list, skip it diff --git a/versions/tests/test_managers.py b/versions/tests/test_managers.py index 396230f0..cb93274d 100644 --- a/versions/tests/test_managers.py +++ b/versions/tests/test_managers.py @@ -135,3 +135,231 @@ def test_version_dropdown_strict( def test_active_file_manager(version, inactive_version): assert Version.objects.active().count() == 1 assert VersionFile.objects.active().count() == 1 + + +def test_default_manager_filters_fully_imported(version, not_imported_version): + """Test that the default manager only returns fully_imported=True objects.""" + # Default queryset should only include fully imported versions + default_versions = Version.objects.all() + assert version in default_versions + assert not_imported_version not in default_versions + + # Count should reflect this + assert default_versions.count() == 1 + + +def test_with_partials_manager_method(version, not_imported_version): + """Test that with_partials() returns all objects regardless of fully_imported status.""" + # with_partials should include all versions + all_versions = Version.objects.with_partials().all() + assert version in all_versions + assert not_imported_version in all_versions + + # Should have more (or equal) count than default + default_count = Version.objects.all().count() + partials_count = all_versions.count() + assert partials_count >= default_count + + +def test_with_partials_active_method(): + """Test that with_partials().active() works correctly.""" + from model_bakery import baker + + # Create test versions with different combinations + active_imported = baker.make( + "versions.Version", name="test-1.0.0", fully_imported=True, active=True + ) + active_not_imported = baker.make( + "versions.Version", name="test-2.0.0", fully_imported=False, active=True + ) + inactive_imported = baker.make( + "versions.Version", name="test-3.0.0", fully_imported=True, active=False + ) + inactive_not_imported = baker.make( + "versions.Version", name="test-4.0.0", fully_imported=False, active=False + ) + + # Default active() should only show active + fully_imported + default_active = Version.objects.active() + assert active_imported in default_active + assert active_not_imported not in default_active + assert inactive_imported not in default_active + assert inactive_not_imported not in default_active + + # with_partials().active() should show both active versions regardless of fully_imported + partials_active = Version.objects.with_partials().active() + assert active_imported in partials_active + assert active_not_imported in partials_active + assert inactive_imported not in partials_active + assert inactive_not_imported not in partials_active + + +def test_with_partials_can_be_chained(): + """Test that with_partials() returns a queryset that can be further filtered.""" + from model_bakery import baker + + # Create test versions + fully_imported_active = baker.make( + "versions.Version", name="test-1.0.0", fully_imported=True, active=True + ) + not_fully_imported_active = baker.make( + "versions.Version", name="test-2.0.0", fully_imported=False, active=True + ) + not_fully_imported_inactive = baker.make( + "versions.Version", name="test-3.0.0", fully_imported=False, active=False + ) + + # Using with_partials() and then filtering + active_with_partials = Version.objects.with_partials().filter(active=True) + assert fully_imported_active in active_with_partials + assert not_fully_imported_active in active_with_partials + assert not_fully_imported_inactive not in active_with_partials + + +def test_with_partials_ordering_and_filtering(): + """Test that with_partials() works with ordering and complex filtering.""" + from model_bakery import baker + import datetime + + today = datetime.date.today() + yesterday = today - datetime.timedelta(days=1) + + # Create test versions with different dates + v1 = baker.make( + "versions.Version", + name="boost-1.0.0", + fully_imported=True, + active=True, + release_date=yesterday, + ) + v2 = baker.make( + "versions.Version", + name="boost-2.0.0", + fully_imported=False, + active=True, + release_date=today, + ) + v3 = baker.make( + "versions.Version", + name="boost-3.0.0", + fully_imported=True, + active=False, + release_date=today, + ) + + # Test ordering with partials + ordered_partials = Version.objects.with_partials().order_by("-release_date") + ordered_list = list(ordered_partials) + + # Should include all versions and be properly ordered + assert len(ordered_list) >= 3 + assert v2 in ordered_list + assert v1 in ordered_list + assert v3 in ordered_list + + # Test filtering with partials + active_partials = Version.objects.with_partials().filter(active=True) + assert v1 in active_partials + assert v2 in active_partials + assert v3 not in active_partials + + +@pytest.mark.django_db +def test_backward_compatibility_existing_methods( + version, not_imported_version, beta_version +): + """Test that existing manager methods work correctly with the new default filtering.""" + # Ensure test versions are set up correctly + version.fully_imported = True + version.save() + beta_version.fully_imported = True + beta_version.save() + not_imported_version.fully_imported = False + not_imported_version.save() + + # active() should work as before (only fully imported active versions) + active_versions = Version.objects.active() + assert version in active_versions + assert not_imported_version not in active_versions + + # most_recent() should work as before + most_recent = Version.objects.most_recent() + assert most_recent is not None + assert most_recent.fully_imported is True + + # most_recent_beta() should work as before + most_recent_beta = Version.objects.most_recent_beta() + assert most_recent_beta is not None + assert most_recent_beta.fully_imported is True + + +@pytest.mark.django_db +def test_get_dropdown_versions_with_partials(): + """Test that get_dropdown_versions works correctly with the new default filtering.""" + from model_bakery import baker + + # Create versions with different fully_imported states + full_release = baker.make( + "versions.Version", + name="boost-1.84.0", + beta=False, + full_release=True, + fully_imported=True, + active=True, + ) + partial_release = baker.make( + "versions.Version", + name="boost-1.85.0", + beta=False, + full_release=True, + fully_imported=False, + active=True, + ) + + # get_dropdown_versions should only include fully imported versions by default + dropdown_versions = Version.objects.get_dropdown_versions() + assert full_release in dropdown_versions + assert partial_release not in dropdown_versions + + # But with_partials should allow access to all versions for custom use cases + all_versions_dropdown = ( + Version.objects.with_partials() + .filter(active=True, beta=False, full_release=True) + .order_by("-name") + ) + assert full_release in all_versions_dropdown + assert partial_release in all_versions_dropdown + + +@pytest.mark.django_db +def test_count_operations(): + """Test count operations with default filtering and with_partials.""" + from model_bakery import baker + + # Create test data + baker.make("versions.Version", name="test-1", fully_imported=True, active=True) + baker.make("versions.Version", name="test-2", fully_imported=True, active=False) + baker.make("versions.Version", name="test-3", fully_imported=False, active=True) + baker.make("versions.Version", name="test-4", fully_imported=False, active=False) + + # Count with default filtering + default_count = Version.objects.count() + + # Count with partials + partials_count = Version.objects.with_partials().count() + + # Count active with default filtering + active_count = Version.objects.active().count() + + # Count active with partials + active_partials_count = Version.objects.with_partials().active().count() + + # Assertions + assert partials_count >= default_count # partials should include more or equal + assert default_count >= active_count # default should include active + assert ( + active_partials_count >= active_count + ) # partials active should include more or equal + assert ( + partials_count >= active_partials_count + ) # all partials should be >= active partials