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

Attr getter works only with zero nested level #56

Closed
nightblure opened this issue Jul 17, 2024 · 5 comments · Fixed by #57
Closed

Attr getter works only with zero nested level #56

nightblure opened this issue Jul 17, 2024 · 5 comments · Fixed by #57

Comments

@nightblure
Copy link
Contributor

nightblure commented Jul 17, 2024

Hey!

I found some problem in attr getter. It lies in the fact that it does not know how to work with nested attributes. Just look at some tests below and you'll understand what we're talking about.


from dataclasses import dataclass, field

import pytest

from that_depends import providers


@dataclass
class Nested2:
    some_const = 144


@dataclass
class Nested1:
    nested2_attr: Nested2 = field(default_factory=Nested2)


@dataclass
class Settings:
    some_str_value: str = 'some_string_value'
    some_int_value: int = 3453621
    nested1_attr: Nested1 = field(default_factory=Nested1)


@pytest.fixture
def some_settings_provider() -> providers.Singleton:
    provider = providers.Singleton(Settings)
    return provider


# WORKS
def test_attr_getter_with_zero_attribute_depth(some_settings_provider):
    attr_getter = some_settings_provider.some_str_value
    assert attr_getter.sync_resolve() == Settings().some_str_value


# DOESNT WORKS BECAUSE Settings HAS NESTED OBJECTS AND ATTRIBUTES
def test_attr_getter_with_more_than_zero_attribute_depth(some_settings_provider):
    attr_getter = some_settings_provider.nested1_attr.nested2_attr.some_const
    assert attr_getter.sync_resolve() == Nested2().some_const

I think I can fix this if you want these cases to work

@lesnik512
Copy link
Member

@nightblure thank you! interesting case. Will think about it

@lesnik512
Copy link
Member

@nightblure will it be hard? I'm not sure, that this feature will be popular, but I don't mind if you want to do it

@nightblure
Copy link
Contributor Author

@nightblure will it be hard? I'm not sure, that this feature will be popular, but I don't mind if you want to do it

I don’t know about popularity, but I think it’s necessary for reasons of convenience

Let's imagine that your service actually has a lot of environment variables and other configuration data. This is usually stored in the form of dataclasses or identity models obtained as a result of parsing environment variables and various configuration files (for example, yaml configs, etc...). And in order not to store a bunch of fields in one class, you can often find division into subclasses for reasons of convenience and logical grouping of variables. Based on this, in some cases this functionality would be necessary. Otherwise, we would have to make a bunch of small providers instead of one or another kind of refactoring

@nightblure
Copy link
Contributor Author

@lesnik512
Copy link
Member

@nightblure valid points

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants