diff --git a/git/objects/submodule/base.py b/git/objects/submodule/base.py index 651d9535c..49dfedf9a 100644 --- a/git/objects/submodule/base.py +++ b/git/objects/submodule/base.py @@ -1401,7 +1401,7 @@ def iter_items( pc = repo.commit(parent_commit) # Parent commit instance parser = cls._config_parser(repo, pc, read_only=True) except (IOError, BadName): - return iter([]) + return # END handle empty iterator for sms in parser.sections(): diff --git a/git/remote.py b/git/remote.py index 4055dba2e..98a421b3a 100644 --- a/git/remote.py +++ b/git/remote.py @@ -130,7 +130,7 @@ def to_progress_instance( return progress -class PushInfo(IterableObj, object): +class PushInfo(IterableObj): """ Carries information about the result of a push operation of a single head:: @@ -300,7 +300,7 @@ def raise_if_error(self) -> None: raise self.error -class FetchInfo(IterableObj, object): +class FetchInfo(IterableObj): """ Carries information about the results of a fetch operation of a single head:: diff --git a/git/util.py b/git/util.py index 0a5da7d71..5acc001f7 100644 --- a/git/util.py +++ b/git/util.py @@ -1183,7 +1183,8 @@ def __delitem__(self, index: Union[SupportsIndex, int, slice, str]) -> None: class IterableClassWatcher(type): - """Metaclass that watches.""" + """Metaclass that issues :class:`DeprecationWarning` when :class:`git.util.Iterable` + is subclassed.""" def __init__(cls, name: str, bases: Tuple, clsdict: Dict) -> None: for base in bases: @@ -1199,23 +1200,42 @@ def __init__(cls, name: str, bases: Tuple, clsdict: Dict) -> None: class Iterable(metaclass=IterableClassWatcher): - """Defines an interface for iterable items, so there is a uniform way to retrieve - and iterate items within the git repository.""" + """Deprecated, use :class:`IterableObj` instead. + + Defines an interface for iterable items, so there is a uniform way to retrieve + and iterate items within the git repository. + """ __slots__ = () _id_attribute_ = "attribute that most suitably identifies your instance" @classmethod - def list_items(cls, repo: "Repo", *args: Any, **kwargs: Any) -> Any: + def iter_items(cls, repo: "Repo", *args: Any, **kwargs: Any) -> Any: + # return typed to be compatible with subtypes e.g. Remote + """Deprecated, use :class:`IterableObj` instead. + + Find (all) items of this type. + + Subclasses can specify ``args`` and ``kwargs`` differently, and may use them for + filtering. However, when the method is called with no additional positional or + keyword arguments, subclasses are obliged to to yield all items. + + :return: Iterator yielding Items """ - Deprecated, use IterableObj instead. + raise NotImplementedError("To be implemented by Subclass") + + @classmethod + def list_items(cls, repo: "Repo", *args: Any, **kwargs: Any) -> Any: + """Deprecated, use :class:`IterableObj` instead. + + Find (all) items of this type and collect them into a list. - Find all items of this type - subclasses can specify args and kwargs differently. - If no args are given, subclasses are obliged to return all items if no additional - arguments arg given. + For more information about the arguments, see :meth:`list_items`. - :note: Favor the iter_items method as it will + :note: Favor the :meth:`iter_items` method as it will avoid eagerly collecting + all items. When there are many items, that can slow performance and increase + memory usage. :return: list(Item,...) list of item instances """ @@ -1223,15 +1243,6 @@ def list_items(cls, repo: "Repo", *args: Any, **kwargs: Any) -> Any: out_list.extend(cls.iter_items(repo, *args, **kwargs)) return out_list - @classmethod - def iter_items(cls, repo: "Repo", *args: Any, **kwargs: Any) -> Any: - # return typed to be compatible with subtypes e.g. Remote - """For more information about the arguments, see list_items. - - :return: Iterator yielding Items - """ - raise NotImplementedError("To be implemented by Subclass") - @runtime_checkable class IterableObj(Protocol): @@ -1246,13 +1257,30 @@ class IterableObj(Protocol): _id_attribute_: str @classmethod - def list_items(cls, repo: "Repo", *args: Any, **kwargs: Any) -> IterableList[T_IterableObj]: + @abstractmethod + def iter_items(cls, repo: "Repo", *args: Any, **kwargs: Any) -> Iterator[T_IterableObj]: + # Return-typed to be compatible with subtypes e.g. Remote. + """Find (all) items of this type. + + Subclasses can specify ``args`` and ``kwargs`` differently, and may use them for + filtering. However, when the method is called with no additional positional or + keyword arguments, subclasses are obliged to to yield all items. + + For more information about the arguments, see list_items. + + :return: Iterator yielding Items """ - Find all items of this type - subclasses can specify args and kwargs differently. - If no args are given, subclasses are obliged to return all items if no additional - arguments arg given. + raise NotImplementedError("To be implemented by Subclass") + + @classmethod + def list_items(cls, repo: "Repo", *args: Any, **kwargs: Any) -> IterableList[T_IterableObj]: + """Find (all) items of this type and collect them into a list. + + For more information about the arguments, see :meth:`list_items`. - :note: Favor the iter_items method as it will + :note: Favor the :meth:`iter_items` method as it will avoid eagerly collecting + all items. When there are many items, that can slow performance and increase + memory usage. :return: list(Item,...) list of item instances """ @@ -1260,16 +1288,6 @@ def list_items(cls, repo: "Repo", *args: Any, **kwargs: Any) -> IterableList[T_I out_list.extend(cls.iter_items(repo, *args, **kwargs)) return out_list - @classmethod - @abstractmethod - def iter_items(cls, repo: "Repo", *args: Any, **kwargs: Any) -> Iterator[T_IterableObj]: # Iterator[T_IterableObj]: - # Return-typed to be compatible with subtypes e.g. Remote. - """For more information about the arguments, see list_items. - - :return: Iterator yielding Items - """ - raise NotImplementedError("To be implemented by Subclass") - # } END classes diff --git a/test/test_submodule.py b/test/test_submodule.py index 4dc89f98f..993f6b57e 100644 --- a/test/test_submodule.py +++ b/test/test_submodule.py @@ -688,6 +688,18 @@ def test_root_module(self, rwrepo): # gitdb: has either 1 or 2 submodules depending on the version. assert len(nsm.children()) >= 1 and nsmc.module_exists() + def test_iter_items_from_nonexistent_hash(self): + it = Submodule.iter_items(self.rorepo, "b4ecbfaa90c8be6ed6d9fb4e57cc824663ae15b4") + with self.assertRaisesRegex(ValueError, r"\bcould not be resolved\b"): + next(it) + + def test_iter_items_from_invalid_hash(self): + """Check legacy behavaior on BadName (also applies to IOError, i.e. OSError).""" + it = Submodule.iter_items(self.rorepo, "xyz") + with self.assertRaises(StopIteration) as ctx: + next(it) + self.assertIsNone(ctx.exception.value) + @with_rw_repo(k_no_subm_tag, bare=False) def test_first_submodule(self, rwrepo): assert len(list(rwrepo.iter_submodules())) == 0