Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to track removed collections in collection-meta.yaml #173

Merged
merged 6 commits into from
Sep 23, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions changelogs/fragments/173-schema-removal.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
minor_changes:
- "Allow information on removed collections in collection metadata schema (https://github.com/ansible-community/antsibull-core/pull/173)."
73 changes: 67 additions & 6 deletions src/antsibull_core/collection_meta.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,19 @@

import os
import typing as t
from collections.abc import Collection

import pydantic as p
from antsibull_fileutils.yaml import load_yaml_file

from .pydantic import forbid_extras, get_formatted_error_messages
from .schemas.collection_meta import (
BaseRemovalInformation,
CollectionMetadata,
CollectionsMetadata,
RemovalInformation,
RemovedCollectionMetadata,
RemovedRemovalInformation,
)

if t.TYPE_CHECKING:
Expand All @@ -42,6 +46,12 @@ def __init__(self, *, all_collections: list[str], major_release: int):
self.all_collections = all_collections
self.major_release = major_release

def _validate_removal_base(
self, collection: str, removal: BaseRemovalInformation, prefix: str
) -> None:
if removal.reason == "renamed" and removal.new_name == collection:
self.errors.append(f"{prefix} new_name: Must not be the collection's name")

def _validate_removal(
self, collection: str, removal: RemovalInformation, prefix: str
) -> None:
Expand All @@ -63,8 +73,7 @@ def _validate_removal(
f" must not be larger than the current major version {self.major_release}"
)

if removal.reason == "renamed" and removal.new_name == collection:
self.errors.append(f"{prefix} new_name: Must not be the collection's name")
self._validate_removal_base(collection, removal, prefix)

def _validate_collection(
self, collection: str, meta: CollectionMetadata, prefix: str
Expand All @@ -75,19 +84,53 @@ def _validate_collection(
if meta.removal:
self._validate_removal(collection, meta.removal, f"{prefix} removal ->")

def validate(self, data: CollectionsMetadata) -> None:
def _validate_removal_for_removed(
self, collection: str, removal: RemovedRemovalInformation, prefix: str
) -> None:
if removal.version.major != self.major_release:
self.errors.append(
f"{prefix} version: Major version of removal version {removal.version} must"
f" be current major version {self.major_release}"
)

if (
removal.announce_version is not None
and removal.announce_version.major >= self.major_release
):
self.errors.append(
f"{prefix} announce_version: Major version of {removal.announce_version}"
f" must be less than the current major version {self.major_release}"
)

self._validate_removal_base(collection, removal, prefix)

def _validate_removed_collection(
self, collection: str, meta: RemovedCollectionMetadata, prefix: str
) -> None:
if meta.repository is None:
self.errors.append(f"{prefix} repository: Required field not provided")

self._validate_removal_for_removed(
collection, meta.removal, f"{prefix} removal ->"
)

def _validate_order(self, collection: Collection, what: str) -> None:
# Check order
sorted_list = sorted(data.collections)
raw_list = list(data.collections)
sorted_list = sorted(collection)
raw_list = list(collection)
if raw_list != sorted_list:
for raw_entry, sorted_entry in zip(raw_list, sorted_list):
if raw_entry != sorted_entry:
self.errors.append(
"The collection list must be sorted; "
f"{what} must be sorted; "
f"{sorted_entry!r} must come before {raw_entry}"
)
break

def _validate_collections(self, data: CollectionsMetadata) -> None:
# Check order
self._validate_order(data.collections, "The collection list")

# Validate collection data
remaining_collections = set(self.all_collections)
for collection, meta in data.collections.items():
Expand All @@ -105,6 +148,24 @@ def validate(self, data: CollectionsMetadata) -> None:
for collection in sorted(remaining_collections):
self.errors.append(f"collections: No metadata present for {collection}")

def _validate_removed_collections(self, data: CollectionsMetadata) -> None:
# Check order
self._validate_order(data.removed_collections, "The removed collection list")

# Validate removed collection data
for collection, removed_meta in data.removed_collections.items():
if collection in self.all_collections:
self.errors.append(
f"removed_collections -> {collection}: Collection in ansible.in"
)
self._validate_removed_collection(
collection, removed_meta, f"removed_collections -> {collection} ->"
)

def validate(self, data: CollectionsMetadata) -> None:
self._validate_collections(data)
self._validate_removed_collections(data)


def lint_collection_meta(
*, collection_meta_path: StrPath, major_release: int, all_collections: list[str]
Expand Down
131 changes: 108 additions & 23 deletions src/antsibull_core/schemas/collection_meta.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,14 @@ def _convert_pypi_version(v: t.Any) -> t.Any:
PydanticPypiVersion = Annotated[PypiVer, BeforeValidator(_convert_pypi_version)]


class RemovalInformation(p.BaseModel):
class BaseRemovalInformation(p.BaseModel):
"""
Stores metadata on when and why a collection will get removed.
Stores metadata on why a collection was/will get removed.
"""

model_config = p.ConfigDict(arbitrary_types_allowed=True)

major_version: t.Union[int, t.Literal["TBD"]]
# The reason because of which the collection will be removed.
reason: t.Literal[
"deprecated",
"considered-unmaintained",
Expand All @@ -60,9 +60,19 @@ class RemovalInformation(p.BaseModel):
"other",
]
reason_text: t.Optional[str] = None

# The Ansible version in which the announcement was made. This is needed
# for changelog generation.
announce_version: t.Optional[PydanticPypiVersion] = None

# In case reason=renamed, the new name of the collection.
new_name: t.Optional[str] = None

# The link to the discussion of the removal.
discussion: t.Optional[p.HttpUrl] = None

# In case reason=renamed, the major Ansible release in which the collection's
# contents have been replaced by deprecated redirects.
redirect_replacement_major_version: t.Optional[int] = None

@p.model_validator(mode="after") # pyre-ignore[56]
Expand All @@ -80,60 +90,135 @@ def _check_reason_text(self) -> Self:
)
return self

@p.model_validator(mode="after") # pyre-ignore[56]
def _check_renamed_base(self) -> Self:
if self.reason == "renamed":
if self.new_name is None:
raise ValueError("new_name must be provided if reason is 'renamed'")
else:
if self.new_name is not None:
raise ValueError(
"new_name must not be provided if reason is not 'renamed'"
)
if self.redirect_replacement_major_version is not None:
raise ValueError(
"redirect_replacement_major_version must not be provided"
" if reason is not 'renamed'"
)
return self


class RemovalInformation(BaseRemovalInformation):
"""
Stores metadata on when and why a collection will get removed.
"""

# The Ansible major version from which the collection will be removed.
major_version: t.Union[int, t.Literal["TBD"]]

@p.model_validator(mode="after") # pyre-ignore[56]
def _check_renamed(self) -> Self:
if self.reason == "renamed":
if (
self.redirect_replacement_major_version is not None
and self.major_version != "TBD"
and self.redirect_replacement_major_version
>= self.major_version # pyre-ignore[58]
):
raise ValueError(
"redirect_replacement_major_version must be smaller than major_version"
)
else:
if self.major_version == "TBD":
raise ValueError(
"major_version must not be TBD if reason is not 'renamed'"
)
return self


class RemovedRemovalInformation(BaseRemovalInformation):
"""
Stores metadata on when and why a collection was removed.
"""

# The exact version from which the collection has been removed.
# This is needed for changelog generation.
version: PydanticPypiVersion

@p.model_validator(mode="after") # pyre-ignore[56]
def _check_reason_is_renamed(self) -> Self:
if self.reason != "renamed":
return self
if self.new_name is None:
raise ValueError("new_name must be provided if reason is 'renamed'")
if (
self.redirect_replacement_major_version is not None
and self.major_version != "TBD"
and self.redirect_replacement_major_version
>= self.major_version # pyre-ignore[58]
and self.redirect_replacement_major_version >= self.version.major
):
raise ValueError(
"redirect_replacement_major_version must be smaller than major_version"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This text needs to be adjusted now that the major_version field is no longer there. Also, might be good to add a test case for this error.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

)
return self

@p.model_validator(mode="after") # pyre-ignore[56]
def _check_reason_is_not_renamed(self) -> Self:
if self.reason == "renamed":
return self
if self.new_name is not None:
raise ValueError("new_name must not be provided if reason is not 'renamed'")
if self.redirect_replacement_major_version is not None:
raise ValueError(
"redirect_replacement_major_version must not be provided if reason is not 'renamed'"
)
if self.major_version == "TBD":
raise ValueError("major_version must not be TBD if reason is not 'renamed'")
return self


class CollectionMetadata(p.BaseModel):
class BaseCollectionMetadata(p.BaseModel):
"""
Stores metadata about one collection.
"""

# If the collection does not use changelogs/changelog.yaml, it can provide
# a URL where the collection's changelog can be found.
changelog_url: t.Optional[str] = p.Field(alias="changelog-url", default=None)

# In case the collection is not located in the root of its repository, the
# subdirectory in which the collection appears.
collection_directory: t.Optional[str] = p.Field(
alias="collection-directory", default=None
)

# The collection's repository.
repository: t.Optional[str] = None

# A regular expression to match the collection's version from a tag in the repository.
tag_version_regex: t.Optional[str] = None

# A list of maintainers. These should be usernames for the repository's
# hosting environment.
maintainers: list[str] = []


class CollectionMetadata(BaseCollectionMetadata):
"""
Stores metadata about one collection.
"""

model_config = p.ConfigDict(arbitrary_types_allowed=True)

# Optional information that the collection will be removed from
# a future Ansible release.
removal: t.Optional[RemovalInformation] = None


class RemovedCollectionMetadata(BaseCollectionMetadata):
"""
Stores metadata about a removed collection.
"""

model_config = p.ConfigDict(arbitrary_types_allowed=True)

# Information why the collection has been removed
removal: RemovedRemovalInformation


class CollectionsMetadata(p.BaseModel):
"""
Stores metadata about a set of collections.
"""

# Metadata on the collections included in Ansible.
collections: dict[str, CollectionMetadata]

# Metadata on the collections removed from this major version of Ansible.
removed_collections: dict[str, RemovedCollectionMetadata] = {}

@staticmethod
def load_from(deps_dir: StrPath | None) -> CollectionsMetadata:
if deps_dir is None:
Expand Down
37 changes: 37 additions & 0 deletions tests/functional/test_collection_meta.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,26 @@
reason_text: The collection wasn't cow friendly, so the Steering Committee decided to kick it out.
discussion: https://forum.ansible.com/...
announce_version: 9.3.0
removed_collections:
bad.baz2:
repository: https://github.com/ansible-collections/collection_template
removal:
version: 10.2.1
reason: renamed
new_name: bad.bar2
announce_version: 9.3.0
redirect_replacement_major_version: 7
bad.baz1:
removal:
version: 9.1.0
reason: deprecated
announce_version: 7.1.0
foo.bar:
repository: https://github.com/ansible-collections/collection_template
removal:
version: 9.0.0
reason: deprecated
announce_version: 7.1.0
""",
[
"foo.bar",
Expand All @@ -153,6 +173,7 @@
],
[
"The collection list must be sorted; 'baz.bam' must come before foo.bar",
"The removed collection list must be sorted; 'bad.baz1' must come before bad.baz2",
"collections -> bad.bar1 -> removal -> announce_version: Major version of 10.1.0 must not be larger than the current major version 9",
"collections -> bad.bar1 -> removal -> major_version: Removal major version 7 must be larger than current major version 9",
"collections -> bad.bar1: Collection not in ansible.in",
Expand All @@ -162,6 +183,10 @@
"collections -> baz.bam: Collection not in ansible.in",
"collections -> foo.bar -> repository: Required field not provided",
"collections: No metadata present for not.there",
"removed_collections -> bad.baz1 -> repository: Required field not provided",
"removed_collections -> bad.baz2 -> removal -> announce_version: Major version of 9.3.0 must be less than the current major version 9",
"removed_collections -> bad.baz2 -> removal -> version: Major version of removal version 10.2.1 must be current major version 9",
"removed_collections -> foo.bar: Collection in ansible.in",
],
),
(
Expand Down Expand Up @@ -271,6 +296,16 @@
removal:
major_version: 11
reason: foo
removed_collections:
bad.foo1:
repository: https://github.com/ansible-collections/collection_template
removal:
version: TBD
reason: deprecated
bad.foo2:
repository: https://github.com/ansible-collections/collection_template
removal:
reason: deprecated
extra_stuff: baz
""",
[],
Expand All @@ -292,6 +327,8 @@
"collections -> bad.foo8 -> removal: Value error, new_name must not be provided if reason is not 'renamed'",
"collections -> bad.foo9 -> removal: Value error, redirect_replacement_major_version must not be provided if reason is not 'renamed'",
"extra_stuff: Extra inputs are not permitted",
"removed_collections -> bad.foo1 -> removal -> version: Value error, Invalid version: 'TBD'",
"removed_collections -> bad.foo2 -> removal -> version: Field required",
],
),
]
Expand Down