From 70e4bb82500fba31013e274578ee3627de02afdf Mon Sep 17 00:00:00 2001 From: Sjoerd Langkemper Date: Tue, 12 Oct 2021 11:23:39 +0200 Subject: [PATCH 1/8] Let remote.push return a PushInfoList List-like, so that it's backward compatible. But it has a new method raise_on_error, that throws an exception if anything failed to push. Related to #621 --- git/remote.py | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/git/remote.py b/git/remote.py index 2cf5678b6..63b4dc510 100644 --- a/git/remote.py +++ b/git/remote.py @@ -116,6 +116,22 @@ def to_progress_instance(progress: Union[Callable[..., Any], RemoteProgress, Non return progress +class PushInfoList(IterableList): + def __new__(cls) -> 'IterableList[IterableObj]': + return super(IterableList, cls).__new__(cls, 'push_infos') + + def __init__(self) -> None: + super().__init__('push_infos') + self.exception = None + + def raise_on_error(self): + """ + Raise an exception if any ref failed to push. + """ + if self.exception: + raise self.exception + + class PushInfo(IterableObj, object): """ Carries information about the result of a push operation of a single head:: @@ -774,7 +790,7 @@ def _get_fetch_info_from_stderr(self, proc: 'Git.AutoInterrupt', def _get_push_info(self, proc: 'Git.AutoInterrupt', progress: Union[Callable[..., Any], RemoteProgress, None], - kill_after_timeout: Union[None, float] = None) -> IterableList[PushInfo]: + kill_after_timeout: Union[None, float] = None) -> PushInfoList: progress = to_progress_instance(progress) # read progress information from stderr @@ -782,7 +798,7 @@ def _get_push_info(self, proc: 'Git.AutoInterrupt', # read the lines manually as it will use carriage returns between the messages # to override the previous one. This is why we read the bytes manually progress_handler = progress.new_message_handler() - output: IterableList[PushInfo] = IterableList('push_infos') + output: PushInfoList = PushInfoList() def stdout_handler(line: str) -> None: try: @@ -796,13 +812,14 @@ def stdout_handler(line: str) -> None: stderr_text = progress.error_lines and '\n'.join(progress.error_lines) or '' try: proc.wait(stderr=stderr_text) - except Exception: + except Exception as e: # This is different than fetch (which fails if there is any std_err # even if there is an output) if not output: raise elif stderr_text: log.warning("Error lines received while fetching: %s", stderr_text) + output.exception = e return output From 6fc830ac4ac6150a218ce6c420ad7e325843c9a4 Mon Sep 17 00:00:00 2001 From: Sjoerd Langkemper Date: Wed, 13 Oct 2021 10:03:53 +0200 Subject: [PATCH 2/8] Test that return value of push is a list-like object --- test/test_remote.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/test_remote.py b/test/test_remote.py index 088fdad55..fedfa2070 100644 --- a/test/test_remote.py +++ b/test/test_remote.py @@ -30,7 +30,7 @@ fixture, GIT_DAEMON_PORT ) -from git.util import rmtree, HIDE_WINDOWS_FREEZE_ERRORS +from git.util import rmtree, HIDE_WINDOWS_FREEZE_ERRORS, IterableList import os.path as osp @@ -128,6 +128,9 @@ def _do_test_fetch_result(self, results, remote): # END for each info def _do_test_push_result(self, results, remote): + self.assertIsInstance(results, list) + self.assertIsInstance(results, IterableList) + self.assertGreater(len(results), 0) self.assertIsInstance(results[0], PushInfo) for info in results: From 5c15975a9537ad3598521ba0adc95b8e3502a9e6 Mon Sep 17 00:00:00 2001 From: Sjoerd Langkemper Date: Mon, 8 Nov 2021 16:20:32 +0000 Subject: [PATCH 3/8] Rename exception to error, raise_on_error to raise_if_error --- git/remote.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/git/remote.py b/git/remote.py index 63b4dc510..745436761 100644 --- a/git/remote.py +++ b/git/remote.py @@ -122,14 +122,14 @@ def __new__(cls) -> 'IterableList[IterableObj]': def __init__(self) -> None: super().__init__('push_infos') - self.exception = None + self.error = None - def raise_on_error(self): + def raise_if_error(self): """ Raise an exception if any ref failed to push. """ - if self.exception: - raise self.exception + if self.error: + raise self.error class PushInfo(IterableObj, object): @@ -819,7 +819,7 @@ def stdout_handler(line: str) -> None: raise elif stderr_text: log.warning("Error lines received while fetching: %s", stderr_text) - output.exception = e + output.error = e return output From 808179ecadaef22fe1b8781e11fc0becddc89c11 Mon Sep 17 00:00:00 2001 From: Sjoerd Langkemper Date: Mon, 8 Nov 2021 17:06:37 +0000 Subject: [PATCH 4/8] Test raise_if_error --- test/test_remote.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/test_remote.py b/test/test_remote.py index fedfa2070..761a7a3e7 100644 --- a/test/test_remote.py +++ b/test/test_remote.py @@ -154,6 +154,12 @@ def _do_test_push_result(self, results, remote): # END error checking # END for each info + if any([info.flags & info.ERROR for info in results]): + self.assertRaises(GitCommandError, results.raise_if_error) + else: + # No errors, so this should do nothing + results.raise_if_error() + def _do_test_fetch_info(self, repo): self.assertRaises(ValueError, FetchInfo._from_line, repo, "nonsense", '') self.assertRaises( From e5fd7b71545b42ae9da9943d922341196c405824 Mon Sep 17 00:00:00 2001 From: Sjoerd Langkemper Date: Tue, 9 Nov 2021 11:55:51 +0000 Subject: [PATCH 5/8] Add raise_if_error() to tutorial --- test/test_docs.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/test_docs.py b/test/test_docs.py index 220156bce..8897bbb75 100644 --- a/test/test_docs.py +++ b/test/test_docs.py @@ -393,7 +393,8 @@ def test_references_and_objects(self, rw_dir): origin.rename('new_origin') # push and pull behaves similarly to `git push|pull` origin.pull() - origin.push() + origin.push() # attempt push, ignore errors + origin.push().raise_if_error() # push and raise error if it fails # assert not empty_repo.delete_remote(origin).exists() # create and delete remotes # ![25-test_references_and_objects] From 56a395fb76eac3e5fc629e02a101ed51cc0c8a91 Mon Sep 17 00:00:00 2001 From: Sjoerd Langkemper Date: Tue, 9 Nov 2021 12:17:02 +0000 Subject: [PATCH 6/8] Fix type handing on PushInfoList --- git/remote.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/git/remote.py b/git/remote.py index 745436761..aae845e5d 100644 --- a/git/remote.py +++ b/git/remote.py @@ -117,14 +117,15 @@ def to_progress_instance(progress: Union[Callable[..., Any], RemoteProgress, Non class PushInfoList(IterableList): - def __new__(cls) -> 'IterableList[IterableObj]': - return super(IterableList, cls).__new__(cls, 'push_infos') + def __new__(cls) -> 'PushInfoList': + base = super().__new__(cls, 'push_infos') + return cast(PushInfoList, base) def __init__(self) -> None: super().__init__('push_infos') self.error = None - def raise_if_error(self): + def raise_if_error(self) -> None: """ Raise an exception if any ref failed to push. """ From bc76d4a371432022bbc1ec5c8893c1a56671ad98 Mon Sep 17 00:00:00 2001 From: Sjoerd Langkemper Date: Tue, 9 Nov 2021 15:16:44 +0000 Subject: [PATCH 7/8] Specify type for PushInfoList.error --- git/remote.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git/remote.py b/git/remote.py index aae845e5d..c212f6d28 100644 --- a/git/remote.py +++ b/git/remote.py @@ -123,7 +123,7 @@ def __new__(cls) -> 'PushInfoList': def __init__(self) -> None: super().__init__('push_infos') - self.error = None + self.error: Optional[Exception] = None def raise_if_error(self) -> None: """ From a270deb86c98123497bb1930cded712fe0c8895e Mon Sep 17 00:00:00 2001 From: Sjoerd Langkemper Date: Wed, 10 Nov 2021 12:40:06 +0000 Subject: [PATCH 8/8] Extend IterableList[PushInfo] instead of IterableList --- git/remote.py | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/git/remote.py b/git/remote.py index c212f6d28..7d5918a5a 100644 --- a/git/remote.py +++ b/git/remote.py @@ -116,23 +116,6 @@ def to_progress_instance(progress: Union[Callable[..., Any], RemoteProgress, Non return progress -class PushInfoList(IterableList): - def __new__(cls) -> 'PushInfoList': - base = super().__new__(cls, 'push_infos') - return cast(PushInfoList, base) - - def __init__(self) -> None: - super().__init__('push_infos') - self.error: Optional[Exception] = None - - def raise_if_error(self) -> None: - """ - Raise an exception if any ref failed to push. - """ - if self.error: - raise self.error - - class PushInfo(IterableObj, object): """ Carries information about the result of a push operation of a single head:: @@ -252,6 +235,22 @@ def iter_items(cls, repo: 'Repo', *args: Any, **kwargs: Any raise NotImplementedError +class PushInfoList(IterableList[PushInfo]): + def __new__(cls) -> 'PushInfoList': + return cast(PushInfoList, IterableList.__new__(cls, 'push_infos')) + + def __init__(self) -> None: + super().__init__('push_infos') + self.error: Optional[Exception] = None + + def raise_if_error(self) -> None: + """ + Raise an exception if any ref failed to push. + """ + if self.error: + raise self.error + + class FetchInfo(IterableObj, object): """