PR#1500: python: Don't apply bind_proxy in BaseProxy __init__() - copr/copr

copr / copr

#1500 python: Don't apply bind_proxy in BaseProxy init()

Merged 4 years ago by praiskup. Opened 4 years ago by csomh.

copr/ csomh/copr bind-proxy into master

python: Don't apply bind_proxy in BaseProxy __init__()

Hunor Csomortáni • 4 years ago

fc64a1a

python/copr/test/client_v3/test_helpers.py

file modified

+1 -1

		`@@ -40,7 +40,7 @@`
		`@mock.patch("copr.v3.proxies.build.BuildProxy.get")`
		`def test_wait_custom_list(self, mock_get):`
		`builds = List([Munch(id=1, state="succeeded"), Munch(id=2, state="failed")], proxy=BuildProxy({}))`
		`- mock_get.side_effect = lambda self, id: builds[id-1]`
		`+ mock_get.side_effect = lambda id: builds[id-1]`
		`assert wait(builds)`

		`@mock.patch("time.time")`

python/copr/v3/helpers.py

file modified

+7 -4

		`@@ -41,13 +41,16 @@`
		`return config`


		`- def for_all_methods(cls, decorator):`
		`+ def for_all_methods(decorator):`
praiskup commented 4 years ago This looks weird. Did you mean `for_all_methods(cls)`?
praiskup commented 4 years ago Nope, sorry, that's correct.
		`"""`
		`Apply a given decorator to all class methods`
		`"""`
		`- for attr in list(cls.__dict__):`
		`- if callable(getattr(cls, attr)):`
		`- setattr(cls, attr, decorator(getattr(cls, attr)))`
		`+ def decorate(cls):`
		`+ for attr in list(cls.__dict__):`
		`+ if callable(getattr(cls, attr)):`
		`+ setattr(cls, attr, decorator(getattr(cls, attr)))`
		`+ return cls`
		`+ return decorate`


		`def bind_proxy(func):`

python/copr/v3/proxies/__init__.py

file modified

+1 -1

		`@@ -4,13 +4,13 @@`
		`from ..helpers import for_all_methods, bind_proxy`


		`+ @for_all_methods(bind_proxy)`
		`class BaseProxy(object):`
		`"""`
		`Parent class for all other proxies`
		`"""`

		`def __init__(self, config):`
		`- for_all_methods(self.__class__, bind_proxy)`
		`self.config = config`

		`@classmethod`

python/copr/v3/proxies/build.py

file modified

		`@@ -4,8 +4,10 @@`
		`from . import BaseProxy`
		`from ..requests import Request, FileRequest, munchify, POST`
		`from ..exceptions import CoprValidationException`
		`+ from ..helpers import for_all_methods, bind_proxy`


		`+ @for_all_methods(bind_proxy)`
		`class BuildProxy(BaseProxy):`
		`def get(self, build_id):`
		`"""`

python/copr/v3/proxies/build_chroot.py

file modified

		`@@ -2,8 +2,10 @@`

		`from . import BaseProxy`
		`from ..requests import Request, munchify`
		`+ from ..helpers import for_all_methods, bind_proxy`


		`+ @for_all_methods(bind_proxy)`
		`class BuildChrootProxy(BaseProxy):`
		`def get(self, build_id, chrootname):`
		`"""`

python/copr/v3/proxies/mock_chroot.py

file modified

		`@@ -3,8 +3,10 @@`
		`import os`
		`from . import BaseProxy`
		`from ..requests import Request, munchify`
		`+ from ..helpers import for_all_methods, bind_proxy`


		`+ @for_all_methods(bind_proxy)`
		`class MockChrootProxy(BaseProxy):`

		`def get_list(self, pagination=None):`

python/copr/v3/proxies/module.py

file modified

		`@@ -3,8 +3,10 @@`
		`import os`
		`from . import BaseProxy`
		`from ..requests import Request, FileRequest, munchify, POST`
		`+ from ..helpers import for_all_methods, bind_proxy`


		`+ @for_all_methods(bind_proxy)`
		`class ModuleProxy(BaseProxy):`

		`def build_from_url(self, ownername, projectname, url, branch="master",`

python/copr/v3/proxies/package.py

file modified

		`@@ -2,8 +2,10 @@`
		`from . import BaseProxy`
		`from .build import BuildProxy`
		`from ..requests import Request, munchify, POST`
		`+ from ..helpers import for_all_methods, bind_proxy`


		`+ @for_all_methods(bind_proxy)`
		`class PackageProxy(BaseProxy):`

		`def get(self, ownername, projectname, packagename,`

python/copr/v3/proxies/project.py

file modified

		`@@ -2,8 +2,10 @@`

		`from . import BaseProxy`
		`from ..requests import Request, munchify, POST, GET, PUT`
		`+ from ..helpers import for_all_methods, bind_proxy`


		`+ @for_all_methods(bind_proxy)`
		`class ProjectProxy(BaseProxy):`

		`def get(self, ownername, projectname):`

python/copr/v3/proxies/project_chroot.py

file modified

		`@@ -3,8 +3,10 @@`
		`import os`
		`from . import BaseProxy`
		`from ..requests import Request, FileRequest, munchify, POST`
		`+ from ..helpers import for_all_methods, bind_proxy`


		`+ @for_all_methods(bind_proxy)`
		`class ProjectChrootProxy(BaseProxy):`

		`def get(self, ownername, projectname, chrootname):`

csomh commented 4 years ago

Applying the 'bind_proxy' decorator in the __init__() of BaseProxy on
every method of the class was done to save some effort in repeating a
class decorator.

This approach though results in the 'bind_proxy' decorator being applied
every time a proxy object is constructed, which, if enough objects are
constructed, leads to a RecursionError when the stack of 'bind_proxy'
decorators becomes higher than the maximum recursion depth of the Python
interpreter.

Make 'for_all_methods' a class decorator and use it decorate all the
'*Proxy' classes, instead of calling it in BaseProxy.__init__().

Fixes: #1499.

Signed-off-by: Hunor Csomortáni csomh@redhat.com

csomh commented 4 years ago

Hey @frostyx, let me know if something is missing from here :)