From f1090475dffd1aa20ae77f47043ff94862a08a76 Mon Sep 17 00:00:00 2001 From: Daniel Alley Date: Tue, 7 May 2024 00:57:27 -0400 Subject: [PATCH] Deprecate / phase out use of the is_modular field from the Package model closes #3524 --- CHANGES/3524.deprecation | 1 + pulp_rpm/app/depsolving.py | 13 ++-------- pulp_rpm/app/models/package.py | 4 +-- pulp_rpm/app/models/repository.py | 30 ++++++++++------------ pulp_rpm/app/serializers/package.py | 2 +- pulp_rpm/app/tasks/synchronizing.py | 1 - pulp_rpm/tests/functional/api/test_sync.py | 8 ------ 7 files changed, 19 insertions(+), 40 deletions(-) create mode 100644 CHANGES/3524.deprecation diff --git a/CHANGES/3524.deprecation b/CHANGES/3524.deprecation new file mode 100644 index 000000000..25346a3cb --- /dev/null +++ b/CHANGES/3524.deprecation @@ -0,0 +1 @@ +Deprecate and start phasing out the is_modular flag on Packages - it's not a reliable source of information. \ No newline at end of file diff --git a/pulp_rpm/app/depsolving.py b/pulp_rpm/app/depsolving.py index bfab3f23e..a0d26d31e 100644 --- a/pulp_rpm/app/depsolving.py +++ b/pulp_rpm/app/depsolving.py @@ -669,18 +669,9 @@ def _load_from_version(self, repo_version, as_target=False): "pk" ) - nonmodular_rpms = models.Package.objects.filter( - pk__in=package_ids, is_modular=False - ).values(*RPM_FIELDS) + packages = models.Package.objects.filter(pk__in=package_ids).values(*RPM_FIELDS) - for rpm in nonmodular_rpms.iterator(chunk_size=5000): - self._add_unit_to_solver(rpm_to_solvable, rpm, repo, libsolv_repo_name) - - modular_rpms = models.Package.objects.filter(pk__in=package_ids, is_modular=True).values( - *RPM_FIELDS - ) - - for rpm in modular_rpms.iterator(chunk_size=5000): + for rpm in packages.iterator(chunk_size=5000): self._add_unit_to_solver(rpm_to_solvable, rpm, repo, libsolv_repo_name) # Load modules into the solver diff --git a/pulp_rpm/app/models/package.py b/pulp_rpm/app/models/package.py index aeab1d27a..1eac632cc 100644 --- a/pulp_rpm/app/models/package.py +++ b/pulp_rpm/app/models/package.py @@ -136,7 +136,7 @@ class Package(Content): rpm_header_end (BigInteger): Last byte of the header is_modular (Bool): - Flag to identify if the package is modular + DEPRECATED: Flag to identify if the package is modular size_archive (BigInteger): Size, in bytes, of the archive portion of the original package file @@ -229,7 +229,7 @@ class Package(Content): time_build = models.BigIntegerField(null=True) time_file = models.BigIntegerField(null=True) - # not part of createrepo_c metadata + # not part of createrepo_c metadata - DEPRECATED is_modular = models.BooleanField(default=False) # createrepo_c treats 'nosrc' arch (opensuse specific use) as 'src' so it can seem that two diff --git a/pulp_rpm/app/models/repository.py b/pulp_rpm/app/models/repository.py index bb2d08e20..b3766a6e8 100644 --- a/pulp_rpm/app/models/repository.py +++ b/pulp_rpm/app/models/repository.py @@ -435,25 +435,21 @@ def _apply_retention_policy(self, new_version): ), "Cannot apply retention policy to completed repository versions" if self.retain_package_versions > 0: - # It would be more ideal if, instead of annotating with an age and filtering manually, - # we could use Django to filter the particular Package content we want to delete. - # Something like ".filter(F('age') > self.retain_package_versions)" would be better - # however this is not currently possible with Django. It would be possible with raw - # SQL but the repository version content membership subquery is currently - # django-managed and would be difficult to share. - # - # Instead we have to do the filtering manually. - nonmodular_packages = ( - Package.objects.with_age() - .filter( - pk__in=new_version.content.filter(pulp_type=Package.get_pulp_type()), - is_modular=False, # don't want to filter out modular RPMs - ) - .only("pk") - ) + # This code is likely not as efficient as it could / should be, but it is heavily + # complicated by the lack of a reliable means to determine whether a package is + # modular or not via SQL + packages = new_version.content.filter(pulp_type=Package.get_pulp_type()) + modules = new_version.content.filter(pulp_type=Modulemd.get_pulp_type()) + + modular_packages = set() + for module in Modulemd.objects.filter(pk__in=modules): + modular_packages.update(module.packages.all().values_list("pk", flat=True)) + + nonmodular_packages = set(packages.values_list("pk", flat=True)) - modular_packages + nonmodular_packages = Package.objects.filter(pk__in=nonmodular_packages) old_packages = [] - for package in nonmodular_packages: + for package in nonmodular_packages.with_age().iterator().only("pk", "age"): if package.age > self.retain_package_versions: old_packages.append(package.pk) diff --git a/pulp_rpm/app/serializers/package.py b/pulp_rpm/app/serializers/package.py index 010985258..a1f4f50df 100644 --- a/pulp_rpm/app/serializers/package.py +++ b/pulp_rpm/app/serializers/package.py @@ -198,7 +198,7 @@ class PackageSerializer(SingleArtifactContentUploadSerializer, ContentChecksumSe read_only=True, ) is_modular = serializers.BooleanField( - help_text=_("Flag to identify if the package is modular"), + help_text=_("DEPRECATED: Flag to identify if the package is modular"), required=False, read_only=True, ) diff --git a/pulp_rpm/app/tasks/synchronizing.py b/pulp_rpm/app/tasks/synchronizing.py index c54ea63f9..890908bcd 100644 --- a/pulp_rpm/app/tasks/synchronizing.py +++ b/pulp_rpm/app/tasks/synchronizing.py @@ -1327,7 +1327,6 @@ def verification_and_skip_callback(pkg): # find if a package relates to a modulemd if dc.content.nevra in self.nevra_to_module.keys(): - dc.content.is_modular = True for dc_modulemd in self.nevra_to_module[dc.content.nevra]: dc.extra_data["modulemd_relation"].append(dc_modulemd) dc_modulemd.extra_data["package_relation"].append(dc) diff --git a/pulp_rpm/tests/functional/api/test_sync.py b/pulp_rpm/tests/functional/api/test_sync.py index 9422a9f37..ef216708d 100644 --- a/pulp_rpm/tests/functional/api/test_sync.py +++ b/pulp_rpm/tests/functional/api/test_sync.py @@ -944,10 +944,6 @@ def test_core_metadata(init_and_sync, rpm_package_api): ) assert list(diff) == [], list(diff) - # assert no package is marked modular - for pkg in get_content(repository.to_dict())[RPM_PACKAGE_CONTENT_NAME]: - assert pkg["is_modular"] is False - @pytest.mark.parallel def test_treeinfo_metadata(init_and_sync, rpm_content_distribution_trees_api): @@ -1045,10 +1041,6 @@ def module_obsolete_key(m): diff = dictdiffer.diff(m1, m2, ignore={"pulp_created", "pulp_href"}) assert list(diff) == [], list(diff) - # assert all package from modular repo is marked as modular - for pkg in get_content(repository.to_dict())[RPM_PACKAGE_CONTENT_NAME]: - assert pkg["is_modular"] is True - @pytest.mark.parallel def test_additive_mode(init_and_sync):