python-dependency-injector
python-dependency-injector copied to clipboard
Erroneous initialization of Resources
After upgrading from 4.10.1 to 4.29.2, I noticed that some Resources which remained unitialized in prior version unexpectedly started to initialize during init_resorces() call. The Selector provider initializes all nested resources disregarding selector key:
from dependency_injector import containers, providers
class PostgresAdapter:
def __init__(self, host, port):
print('Postgres initialized:', host, port)
class SQLiteAdapter:
def __init__(self, db_file):
print('Sqlite initialized:', db_file)
def setup_db_adapter(klass, **kwargs):
yield klass(**kwargs)
print('close')
class Container(containers.DeclarativeContainer):
config = providers.Configuration()
database = providers.Selector(
config.db_type,
postgres=providers.Resource(
setup_db_adapter,
klass=PostgresAdapter,
host=config.db_host,
port=config.db_port,
),
sqlite=providers.Resource(
setup_db_adapter,
klass=SQLiteAdapter,
db_file=config.db_file,
),
)
if __name__ == '__main__':
container = Container(
config={
'db_type': 'postgres',
'db_host': 'localhost',
'db_port': 5432,
}
)
container.init_resources()
container.shutdown_resources()
Output:
Postgres initialized: localhost 5432
Sqlite initialized: None
close
close
Expected output:
Postgres initialized: localhost 5432
close
Both resources are initialized even if db_type is unspecified, which is nonsence:
container = Container(config={})
Output:
Postgres initialized: None None
Sqlite initialized: None
close
close
Hello @atten ,
Method container.init_resources() initializes nested resources since version 4.14.0. Here is an entry from the changelog:
Fix an issue with container.init_resource() and container.shutdown_resource() ignoring nested resources that are not present on the root level. See issue: #380.
I'm sorry that this change affected you negatively. I didn't realize the Selector + Resource use case.
If you're looking for a workaround, you could stop calling container.init_resources(). It will make the initialization work lazily.
@atten , I think that container.init_resources() behaviour should keep initialize nested resources by default. This covers a use case when there is nested containers setup.
I could add a parameter to initialize only flat resources container.init_resources(nested=False). How does it look to you?
Or if lazy option with having no container.init_resources() call would work, I could leave it as it's now.
As a user of your package, I need to initialize resources which are enabled by Selector and keep disabled ones untouched (as it worked prior to 4.14.0. What you suggest with nested=False doesn't solve this problem as far as I can see (on a top-level, I dont't need to know whether resources are nested or not).
The example in the first message demonstrates a program with 2 databases (production-ready and testing) with a selector. Do you think that attempt of initializing both of them disregarding of specific config/environment is a good idea?
The example in the first message demonstrates a program with 2 databases (production-ready and testing) with a selector. Do you think that attempt of initializing both of them disregarding of specific config/environment is a good idea?
No. It's not a good idea.
Selector and resource providers do not work the same way you described. Method container.init_resources() should initialize all resources in the container. Prior to version 4.10 this method didn't initialize nested resources. So if you redesign your example a bit you will get in the same trouble like with latest version:
class Container(containers.DeclarativeContainer):
config = providers.Configuration()
postgres_db = providers.Resource(
setup_db_adapter,
klass=PostgresAdapter,
host=config.db_host,
port=config.db_port,
)
sqlite_db = providers.Resource(
setup_db_adapter,
klass=SQLiteAdapter,
db_file=config.db_file,
)
database = providers.Selector(
config.db_type,
postgres=postgres_db,
sqlite=sqlite_db,
)
if __name__ == '__main__':
container = Container(
config={
'db_type': 'postgres',
'db_host': 'localhost',
'db_port': 5432,
}
)
container.init_resources() # 4.10.1: All resources initialized, not desired behaviour
container.shutdown_resources()
Output:
Postgres initialized: localhost 5432
Sqlite initialized: None
close
close
Is it a bug or feature? Experience shows that it's a tricky question. Despite somebody finds it to be a bug, I consider your use case totally valid.
From the point of the library, method container.init_resources() should initialize all container resources. That's what happens now. I think that it's a fair statement.
Finally, speaking about your use case, I would recommend to redesign it that way:
from dependency_injector import containers, providers
class PostgresAdapter:
def __init__(self, host, port):
print('Postgres initialized:', host, port)
class SQLiteAdapter:
def __init__(self, db_file):
print('Sqlite initialized:', db_file)
def setup_db_adapter(adapter):
yield adapter
print('close')
class Container(containers.DeclarativeContainer):
config = providers.Configuration()
database = providers.Resource(
setup_db_adapter,
adapter=providers.Selector(
config.db_type,
postgres=providers.Factory(
PostgresAdapter,
host=config.db_host,
port=config.db_port,
),
sqlite=providers.Factory(
SQLiteAdapter,
db_file=config.db_file,
),
),
)
if __name__ == '__main__':
container = Container(
config={
'db_type': 'postgres',
'db_host': 'localhost',
'db_port': 5432,
},
)
container.init_resources()
container.shutdown_resources()
That example works like you expect. The redesign idea is to have single resource. I use selector as a resource dependency. The selector creates proper adapter and passes it to further initialization to the setup generator.
Does it make better sense to you?
Yes it does, thanks!
Your redesign proposal works as expected. However, the idea that init_resources() initializes ALL of them (including disabled by Selector) still seems to be too straightforward. Are resources in overridden containers initialized too?
Are resources in overridden containers initialized too?
Yep.
However, the idea that init_resources() initializes ALL of them (including disabled by Selector) still seems to be too straightforward.
I understand. In fact selector does not "disable" a resource. It just selects the proper resource in runtime. At least this is how current implementation works.
As a short term plan, I can add example above to the resources docs. Long term, need to think of the design. Idea of disabling selector looks interesting. Fairly saying it even could be a new provider ResourceSelector.
Hm this gets a bit unergonomic when I for example want to switch between a resource and a factory. To visualize the problem:
class SecretManager(Protocol):
async def get(self, name: str) -> Secret[str]:
...
class InMemorySecretManager(SecretManager):
"""In-memory mapping of secrets and their values"""
def __init__(self, secrets: dict[str, str]) -> None:
self.secrets = MappingProxyType(secrets)
class SSMSecretManager(SecretManager):
"""Backed by AWS Simple System manager"""
def __init__(self, client: AioBaseClient) -> None:
self.client = client
async def create_ssm_secret_manager(
region_name: str,
aws_access_key_id: str,
aws_secret_access_key: str,
) -> SecretManager:
session = aiobotocore.session.get_session()
client_creator = session.create_client(
"ssm",
region_name=region_name,
aws_access_key_id=aws_access_key_id,
aws_secret_access_key=aws_secret_access_key,
)
async with client_creator as client:
yield SSMSecretManager(client)
class Container(containers.DeclarativeContainer):
settings = providers.Configuration()
secret_manager: providers.Provider[SecretManager] = providers.Selector(
settings.secret_manager.type,
ssm=providers.Resource(
create_ssm_secret_manager,
region_name=settings.secret_manager.region_name.required(),
aws_access_key_id=settings.secret_manager.aws_access_key_id.required(),
aws_secret_access_key=settings.secret_manager.aws_secret_access_key.required(),
),
memory=providers.Factory(
InMemorySecretManager,
secrets=settings.secret_manager.secrets.required(),
),
)
Now when using this with the memory-backed secret manager, the container will complain about missing config for the SSM-backed manager.
If you're looking for a workaround, you could stop calling container.init_resources(). It will make the initialization work lazily.
I feel like a possible solution would be to make init_resources() respect the selectors and only initialize the selected resources. Then in case of an override, new resources would be initialized lazily.