From b047e83c26530cbb56674f46552969387bcf4456 Mon Sep 17 00:00:00 2001 From: Johan Brandhorst Date: Wed, 15 Mar 2017 12:15:51 +0000 Subject: [PATCH 1/7] Stop listing if recursion limit is hit Fixes #217 --- gitlab/__init__.py | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/gitlab/__init__.py b/gitlab/__init__.py index 119dab080..8ede934e1 100644 --- a/gitlab/__init__.py +++ b/gitlab/__init__.py @@ -304,6 +304,8 @@ def _raw_get(self, path_, content_type=None, streamed=False, **kwargs): return self.session.get(url, params=kwargs, stream=streamed, **opts) except Exception as e: + if str(e) == "maximum recursion depth exceeded": + raise e raise GitlabConnectionError( "Can't connect to GitLab server (%s)" % e) @@ -333,11 +335,16 @@ def _raw_list(self, path_, cls, extra_attrs={}, **kwargs): results = [cls(self, item, **params) for item in r.json() if item is not None] - if ('next' in r.links and 'url' in r.links['next'] - and get_all_results is True): - args = kwargs.copy() - args['next_url'] = r.links['next']['url'] - results.extend(self.list(cls, **args)) + try: + if ('next' in r.links and 'url' in r.links['next'] + and get_all_results is True): + args = kwargs.copy() + args['next_url'] = r.links['next']['url'] + results.extend(self.list(cls, **args)) + except Exception as e: + if str(e) != "maximum recursion depth exceeded": + raise e + return results def _raw_post(self, path_, data=None, content_type=None, **kwargs): From 6c8bee0941b123239b75d3607e35c6951783dca5 Mon Sep 17 00:00:00 2001 From: Johan Brandhorst Date: Tue, 21 Mar 2017 21:33:17 +0000 Subject: [PATCH 2/7] Add safe_all parameter to list functions. This new parameter works just like "all", except it will attempt to catch any recursion limit exceptions thrown. These errors could be countered if a large amount of values were being listed recursively. --- gitlab/__init__.py | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/gitlab/__init__.py b/gitlab/__init__.py index 8ede934e1..10e41aa3f 100644 --- a/gitlab/__init__.py +++ b/gitlab/__init__.py @@ -111,8 +111,8 @@ def __init__(self, url, private_token=None, email=None, password=None, # build the "submanagers" for parent_cls in six.itervalues(globals()): if (not inspect.isclass(parent_cls) - or not issubclass(parent_cls, GitlabObject) - or parent_cls == CurrentUser): + or not issubclass(parent_cls, GitlabObject) + or parent_cls == CurrentUser): continue if not parent_cls.managers: @@ -304,8 +304,6 @@ def _raw_get(self, path_, content_type=None, streamed=False, **kwargs): return self.session.get(url, params=kwargs, stream=streamed, **opts) except Exception as e: - if str(e) == "maximum recursion depth exceeded": - raise e raise GitlabConnectionError( "Can't connect to GitLab server (%s)" % e) @@ -313,11 +311,13 @@ def _raw_list(self, path_, cls, extra_attrs={}, **kwargs): params = extra_attrs.copy() params.update(kwargs.copy()) - get_all_results = kwargs.get('all', False) + catch_recursion_limit = kwargs.get('safe_all', False) + get_all_results = kwargs.get( + 'all', False) == True or catch_recursion_limit # Remove these keys to avoid breaking the listing (urls will get too # long otherwise) - for key in ['all', 'next_url']: + for key in ['all', 'next_url', 'safe_all']: if key in params: del params[key] @@ -336,13 +336,16 @@ def _raw_list(self, path_, cls, extra_attrs={}, **kwargs): results = [cls(self, item, **params) for item in r.json() if item is not None] try: - if ('next' in r.links and 'url' in r.links['next'] - and get_all_results is True): - args = kwargs.copy() - args['next_url'] = r.links['next']['url'] - results.extend(self.list(cls, **args)) + if ('next' in r.links and 'url' in r.links['next'] + and get_all_results): + args = kwargs.copy() + args['next_url'] = r.links['next']['url'] + results.extend(self.list(cls, **args)) except Exception as e: - if str(e) != "maximum recursion depth exceeded": + # Catch the recursion limit exception if the 'safe_all' + # kwarg was provided + if not (catch_recursion_limit and + "maximum recursion depth exceeded" in str(e)): raise e return results From 0e8c9262d19cdb7d08735f84d380a4ba7dc13f7d Mon Sep 17 00:00:00 2001 From: Johan Brandhorst Date: Tue, 21 Mar 2017 21:33:48 +0000 Subject: [PATCH 3/7] Update docs to highlight new list parameter --- docs/api-usage.rst | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/api-usage.rst b/docs/api-usage.rst index a15aecbfa..7b7ab7832 100644 --- a/docs/api-usage.rst +++ b/docs/api-usage.rst @@ -142,7 +142,9 @@ parameter to get all the items when using listing methods: python-gitlab will iterate over the list by calling the correspnding API multiple times. This might take some time if you have a lot of items to retrieve. This might also consume a lot of memory as all the items will be - stored in RAM. + stored in RAM. If you're encountering the python recursion limit exception, + use ``safe_all=True`` instead to stop pagination automatically if the + recursion limit is hit. Sudo ==== From 1db23fd32f4b8aa22d958fd65c8b474261813565 Mon Sep 17 00:00:00 2001 From: Johan Brandhorst Date: Tue, 21 Mar 2017 21:33:58 +0000 Subject: [PATCH 4/7] Add tests for new list parameter --- gitlab/tests/test_gitlab.py | 69 +++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/gitlab/tests/test_gitlab.py b/gitlab/tests/test_gitlab.py index 4adf07f5a..0d1ba6e9f 100644 --- a/gitlab/tests/test_gitlab.py +++ b/gitlab/tests/test_gitlab.py @@ -243,6 +243,75 @@ def resp_two(url, request): self.assertEqual(data[0].ref, "b") self.assertEqual(len(data), 2) + def test_list_recursion_limit_caught(self): + @urlmatch(scheme="http", netloc="localhost", + path='/api/v3/projects/1/repository/branches', method="get") + def resp_one(url, request): + """First request: + + http://localhost/api/v3/projects/1/repository/branches?per_page=1 + """ + headers = { + 'content-type': 'application/json', + 'link': '; rel="next", ; rel="las' + 't", ; rel="first"' + } + content = ('[{"branch_name": "otherbranch", ' + '"project_id": 1, "ref": "b"}]').encode("utf-8") + resp = response(200, content, headers, None, 5, request) + return resp + + @urlmatch(scheme="http", netloc="localhost", + path='/api/v3/projects/1/repository/branches', method="get", + query=r'.*page=2.*') + def resp_two(url, request): + # Mock a runtime error + raise RuntimeError("maximum recursion depth exceeded") + + with HTTMock(resp_two, resp_one): + data = self.gl.list(ProjectBranch, project_id=1, per_page=1, + safe_all=True) + self.assertEqual(data[0].branch_name, "otherbranch") + self.assertEqual(data[0].project_id, 1) + self.assertEqual(data[0].ref, "b") + self.assertEqual(len(data), 1) + + def test_list_recursion_limit_not_caught(self): + @urlmatch(scheme="http", netloc="localhost", + path='/api/v3/projects/1/repository/branches', method="get") + def resp_one(url, request): + """First request: + + http://localhost/api/v3/projects/1/repository/branches?per_page=1 + """ + headers = { + 'content-type': 'application/json', + 'link': '; rel="next", ; rel="las' + 't", ; rel="first"' + } + content = ('[{"branch_name": "otherbranch", ' + '"project_id": 1, "ref": "b"}]').encode("utf-8") + resp = response(200, content, headers, None, 5, request) + return resp + + @urlmatch(scheme="http", netloc="localhost", + path='/api/v3/projects/1/repository/branches', method="get", + query=r'.*page=2.*') + def resp_two(url, request): + # Mock a runtime error + raise RuntimeError("maximum recursion depth exceeded") + + with HTTMock(resp_two, resp_one): + with self.assertRaisesRegex(GitlabError, "(maximum recursion depth exceeded)"): + data = self.gl.list(ProjectBranch, project_id=1, per_page=1, + all=True) + def test_list_401(self): @urlmatch(scheme="http", netloc="localhost", path="/api/v3/projects/1/repository/branches", method="get") From 5e681b8a521de217a80fcbbbeb11fbaf76dddcf2 Mon Sep 17 00:00:00 2001 From: Johan Brandhorst Date: Thu, 23 Mar 2017 14:44:45 +0000 Subject: [PATCH 5/7] Make unit tests python version independent --- gitlab/tests/test_gitlab.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/gitlab/tests/test_gitlab.py b/gitlab/tests/test_gitlab.py index 0d1ba6e9f..8b33997e9 100644 --- a/gitlab/tests/test_gitlab.py +++ b/gitlab/tests/test_gitlab.py @@ -27,6 +27,8 @@ from httmock import response # noqa from httmock import urlmatch # noqa +import six + import gitlab from gitlab import * # noqa @@ -308,7 +310,7 @@ def resp_two(url, request): raise RuntimeError("maximum recursion depth exceeded") with HTTMock(resp_two, resp_one): - with self.assertRaisesRegex(GitlabError, "(maximum recursion depth exceeded)"): + with six.assertRaisesRegex(self, GitlabError, "(maximum recursion depth exceeded)"): data = self.gl.list(ProjectBranch, project_id=1, per_page=1, all=True) From 38ea905d32188921fd46266c6f4c6a72d3db92f1 Mon Sep 17 00:00:00 2001 From: Gauvain Pocentek Date: Thu, 23 Mar 2017 19:33:23 +0100 Subject: [PATCH 6/7] pep8 fixes --- gitlab/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gitlab/__init__.py b/gitlab/__init__.py index a59adda3b..421b9eb2c 100644 --- a/gitlab/__init__.py +++ b/gitlab/__init__.py @@ -313,8 +313,8 @@ def _raw_list(self, path_, cls, extra_attrs={}, **kwargs): params.update(kwargs.copy()) catch_recursion_limit = kwargs.get('safe_all', False) - get_all_results = kwargs.get( - 'all', False) == True or catch_recursion_limit + get_all_results = (kwargs.get('all', False) is True + or catch_recursion_limit) # Remove these keys to avoid breaking the listing (urls will get too # long otherwise) From 829ab2ac041bc91a34c41942b95322fc948b4a07 Mon Sep 17 00:00:00 2001 From: Gauvain Pocentek Date: Thu, 23 Mar 2017 19:34:45 +0100 Subject: [PATCH 7/7] pep8 fixes --- gitlab/tests/test_gitlab.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/gitlab/tests/test_gitlab.py b/gitlab/tests/test_gitlab.py index 8b33997e9..4670def2f 100644 --- a/gitlab/tests/test_gitlab.py +++ b/gitlab/tests/test_gitlab.py @@ -26,7 +26,6 @@ from httmock import HTTMock # noqa from httmock import response # noqa from httmock import urlmatch # noqa - import six import gitlab @@ -310,9 +309,9 @@ def resp_two(url, request): raise RuntimeError("maximum recursion depth exceeded") with HTTMock(resp_two, resp_one): - with six.assertRaisesRegex(self, GitlabError, "(maximum recursion depth exceeded)"): - data = self.gl.list(ProjectBranch, project_id=1, per_page=1, - all=True) + with six.assertRaisesRegex(self, GitlabError, + "(maximum recursion depth exceeded)"): + self.gl.list(ProjectBranch, project_id=1, per_page=1, all=True) def test_list_401(self): @urlmatch(scheme="http", netloc="localhost",