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

Aliasing a concrete type causes a RecursionError for identity map #159

Closed
mrogaski opened this issue May 26, 2021 · 4 comments
Closed

Aliasing a concrete type causes a RecursionError for identity map #159

mrogaski opened this issue May 26, 2021 · 4 comments
Labels
bug Something isn't working

Comments

@mrogaski
Copy link

mrogaski commented May 26, 2021

It appears that the change to how the Alias.skip_definitions flag is used in #150 introduced a regression. If an alias maps a type to itself, any resolution to the alias will raise a RecursionError.

Python 3.9.4 (tags/v3.9.4:1f2e308, Apr  6 2021, 13:40:21) [MSC v.1928 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import abc
>>> import sys
>>> import lagom
>>> sys.tracebacklimit = 5
>>>
>>> class AbstractFoo(abc.ABC):
...   pass
...
>>> class Foo(AbstractFoo):
...   pass
...
>>> container = lagom.Container()
>>>
>>> container[AbstractFoo] = Foo
>>> container[AbstractFoo]
<__main__.Foo object at 0x000001BF1CA4D700>
>>> container[Foo]
<__main__.Foo object at 0x000001BF1CA4D790>
>>>
>>> container[Foo] = Foo
>>> container[AbstractFoo]
Traceback (most recent call last):
  File "X:\lagom\.venv\lib\site-packages\lagom\definitions.py", line 68, in get_instance
    return container.resolve(
  File "X:\lagom\.venv\lib\site-packages\lagom\container.py", line 237, in resolve
    return definition.get_instance(self)
  File "X:\lagom\.venv\lib\site-packages\lagom\definitions.py", line 68, in get_instance
    return container.resolve(
  File "X:\lagom\.venv\lib\site-packages\lagom\container.py", line 235, in resolve
    definition = self.get_definition(dep_type)
  File "X:\lagom\.venv\lib\site-packages\lagom\container.py", line 352, in get_definition
    definition = self._registered_types.get(dep_type, Unset)
RecursionError: maximum recursion depth exceeded while calling a Python object
>>> container[Foo]
Traceback (most recent call last):
  File "X:\lagom\.venv\lib\site-packages\lagom\definitions.py", line 68, in get_instance
    return container.resolve(
  File "X:\lagom\.venv\lib\site-packages\lagom\container.py", line 237, in resolve
    return definition.get_instance(self)
  File "X:\lagom\.venv\lib\site-packages\lagom\definitions.py", line 68, in get_instance
    return container.resolve(
  File "X:\lagom\.venv\lib\site-packages\lagom\container.py", line 235, in resolve
    definition = self.get_definition(dep_type)
  File "X:\lagom\.venv\lib\site-packages\lagom\container.py", line 352, in get_definition
    definition = self._registered_types.get(dep_type, Unset)
RecursionError: maximum recursion depth exceeded while calling a Python object
>>>

This is Python 3.9.4 and Lagom 1.3.0.

Ultimately, this isn't a huge issue. But it seems that a guard condition is merited so the identity map can be allowed.

@mrogaski
Copy link
Author

Also, thank you for this wonderful package!

@meadsteve
Copy link
Owner

Thanks for the detailed description @mrogaski I'll take a look at this.

@meadsteve meadsteve added the bug Something isn't working label May 27, 2021
meadsteve added a commit that referenced this issue May 27, 2021
@meadsteve
Copy link
Owner

I've just released 1.3.1 that should fix the regression. Let me know if it doesn't. Thanks again for the detailed report.

@mrogaski
Copy link
Author

I can confirm it's fixed for our use case. (We're allowing a type mapping to be overridden in configuration, but setting it to the identity map by default.)

meadsteve added a commit that referenced this issue Aug 16, 2021
meadsteve added a commit that referenced this issue Aug 16, 2021
* failing test

* make container.partial work for instance methods
Fixes issue #159

* version bump

* 'Refactored by Sourcery' (#163)

Co-authored-by: Sourcery AI <>

* reformatting

Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants