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

Configuration raises AttributeError when provider is called #358

Closed
StefanoFrazzetto opened this issue Jan 14, 2021 · 21 comments
Closed

Configuration raises AttributeError when provider is called #358

StefanoFrazzetto opened this issue Jan 14, 2021 · 21 comments
Assignees
Labels

Comments

@StefanoFrazzetto
Copy link

StefanoFrazzetto commented Jan 14, 2021

Hi, I just run into this issue with the Configuration provider. After scratching my head for a bit, I managed to find a workaround, but I was wondering if this is actually a bug or just something wrong I am doing. Any help would be appreciated!

Steps to reproduce

containers.py

from dependency_injector import providers, containers

class MyService(object):
    def __init__(self, **kwargs):
        self.key = kwargs.pop('key')

    def trigger(self):
        pass

class MyDevice(object):
    def __init__(self, **kwargs):
        # doesn't raise an error because it's an instance of
        # dependency_injector.providers.Singleton
        self.service = kwargs.pop('service')

    def do_something(self):
        # raises "AttributeError: 'NoneType' object has no attribute 'get'"
        self.service().trigger()

class ServiceContainer(containers.DeclarativeContainer):
    config = providers.Configuration()

    myservice = providers.Singleton(MyService, config=config.myservice)


class Container(containers.DeclarativeContainer):
    config = providers.Configuration()

    services = providers.Container(ServiceContainer, config=config.services)

    mydevice = providers.Factory(MyDevice)

If I run app.py

import sys

from containers import Container

container = Container()
container.config.from_yaml('config.yaml')
container.init_resources()
container.wire(modules=[sys.modules[__name__]])

mydevice = container.mydevice(service=container.services.myservice)
mydevice.do_something()

with config.yaml

foo:
  bar: 42

it raises the following error

File "/home/stefano/personal/test-error/containers.py", line 15, in do_something
self.service().trigger()
File "src/dependency_injector/providers.pyx", line 168, in dependency_injector.providers.Provider.call
File "src/dependency_injector/providers.pyx", line 2245, in dependency_injector.providers.Singleton._provide
File "src/dependency_injector/providers.pxd", line 550, in dependency_injector.providers.__factory_call
File "src/dependency_injector/providers.pxd", line 536, in dependency_injector.providers.__callable_call
File "src/dependency_injector/providers.pxd", line 495, in dependency_injector.providers.__call
File "src/dependency_injector/providers.pxd", line 387, in dependency_injector.providers.__provide_keyword_args
File "src/dependency_injector/providers.pxd", line 310, in dependency_injector.providers.__get_value
File "src/dependency_injector/providers.pyx", line 168, in dependency_injector.providers.Provider.call
File "src/dependency_injector/providers.pyx", line 1232, in dependency_injector.providers.ConfigurationOption._provide
File "src/dependency_injector/providers.pyx", line 1467, in dependency_injector.providers.Configuration.get
AttributeError: 'NoneType' object has no attribute 'get'

Workaround
To avoid the issue, I have to pass the whole config to ServiceContainer

class ServiceContainer(containers.DeclarativeContainer):
    config = providers.Configuration()

    myservice = providers.Singleton(MyService, config=config.services.myservice)


class Container(containers.DeclarativeContainer):
    config = providers.Configuration()

    services = providers.Container(ServiceContainer, config=config)

    mydevice = providers.Factory(MyDevice)

Running the application now, raises the following (as expected)

File "/home/stefano/personal/test-error/containers.py", line 18, in do_something
self.service().trigger()
File "src/dependency_injector/providers.pyx", line 168, in dependency_injector.providers.Provider.call
File "src/dependency_injector/providers.pyx", line 2245, in dependency_injector.providers.Singleton._provide
File "src/dependency_injector/providers.pxd", line 550, in dependency_injector.providers.__factory_call
File "src/dependency_injector/providers.pxd", line 536, in dependency_injector.providers.__callable_call
File "src/dependency_injector/providers.pxd", line 526, in dependency_injector.providers.__call
File "/home/stefano/personal/test-error/containers.py", line 5, in init
self.key = kwargs.pop('key')
KeyError: 'key'

@rmk135 rmk135 self-assigned this Jan 14, 2021
@rmk135 rmk135 added the bug label Jan 14, 2021
@rmk135
Copy link
Member

rmk135 commented Jan 14, 2021

Hey @StefanoFrazzetto , what version of Dependency Injector do you use?

There were a couple fixes recently. I tried that code sample on latest version 4.8.2 and it seems to be working correctly.

@rmk135
Copy link
Member

rmk135 commented Jan 14, 2021

Btw, containers in initial example look the same way like in workaround. I have tried next example and it also seems to be working correctly:

class ServiceContainer(containers.DeclarativeContainer):
    config = providers.Configuration()

    myservice = providers.Singleton(MyService, key=config.bar)


class Container(containers.DeclarativeContainer):
    config = providers.Configuration()

    services = providers.Container(ServiceContainer, config=config.foo)

    mydevice = providers.Factory(MyDevice)

@StefanoFrazzetto
Copy link
Author

StefanoFrazzetto commented Jan 14, 2021

Hey @rmk135, thanks for the quick reply!

Sorry, I made a mistake when I copy-pasted the first part (I pasted the workaround instead of the issue); I've corrected the initial comment with the issue. I found the error because I was expecting to load something like

services:
  myservice:
    key: baz

but my config file did not contain the 'services' key, e.g.

something:
  foo: baz

By the way, I'm using version 4.8.2.

@rmk135
Copy link
Member

rmk135 commented Jan 15, 2021

Hey @StefanoFrazzetto , got it, thanks for extra info. I see a bug now, fixing.

@rmk135
Copy link
Member

rmk135 commented Jan 15, 2021

Fixed in 4.8.3. @StefanoFrazzetto Many thanks for reporting the issue. Closing it, re-open if needed.

@rmk135 rmk135 closed this as completed Jan 15, 2021
@StefanoFrazzetto
Copy link
Author

Hey @rmk135, thank you very much for the effort with this!

I've updated to v4.9.0, but unfortunately I'm still experincing the issue. I created a gist with the sample code showing two examples: working and broken config.

I've also found another bug where the config provider doesn't raise an error if you try to load a non-existing file, e.g. container.config.from_yaml('i_dont_exist.yaml'). I can open a separate issue for that one, if you prefer.

@rmk135 rmk135 reopened this Jan 15, 2021
@rmk135
Copy link
Member

rmk135 commented Jan 15, 2021

Got it, will take a look later today.

I've also found another bug where the config provider doesn't raise an error if you try to load a non-existing file, e.g. container.config.from_yaml('i_dont_exist.yaml'). I can open a separate issue for that one, if you prefer.

This is not a bug, but a feature. It's done that way to be able to put in a code multiple locations like:

container.config.from_yaml('config.yaml')
container.config.from_yaml('config.local.yaml')

In that case settings from config.local.yaml will override the settings from config.yaml. If config.local.yaml does not exist, no error is expected.

@rmk135
Copy link
Member

rmk135 commented Jan 15, 2021

I have applied one more fix and now sample code provides:

### WORKING
> Device: triggering service
> Service: success

### BUG
> Device: triggering service
Traceback (most recent call last):
  File "/Users/rmoh/projects/ets-labs/python-dependency-injector/.workspace/issue358/bug.py", line 64, in <module>
    execute(broken)
  File "/Users/rmoh/projects/ets-labs/python-dependency-injector/.workspace/issue358/bug.py", line 52, in execute
    mydevice.do_something()
  File "/Users/rmoh/projects/ets-labs/python-dependency-injector/.workspace/issue358/bug.py", line 23, in do_something
    self.service().trigger()
  File "src/dependency_injector/providers.pyx", line 168, in dependency_injector.providers.Provider.__call__
  File "src/dependency_injector/providers.pyx", line 2250, in dependency_injector.providers.Singleton._provide
  File "src/dependency_injector/providers.pxd", line 550, in dependency_injector.providers.__factory_call
  File "src/dependency_injector/providers.pxd", line 536, in dependency_injector.providers.__callable_call
  File "src/dependency_injector/providers.pxd", line 526, in dependency_injector.providers.__call
  File "/Users/rmoh/projects/ets-labs/python-dependency-injector/.workspace/issue358/bug.py", line 8, in __init__
    self.key = config.pop('key')
AttributeError: 'NoneType' object has no attribute 'pop'

While that fixes the internal bug, it does not lead to the fail-fast behaviour. That happens because undefined config value provides none: config.undefined() is None. I guess that desired behaviour here is to see an error that config option is undefined. This is something that will be implemented for #341 . config will go into strict mode: if undefined key is requested, Configuration provider will raise an error. I'll take #341 into work in next couple of days and will update this issue when it's done.

PS: I published a fix that leads to 'NoneType' object has no attribute 'pop' in version 4.9.1.

@rmk135
Copy link
Member

rmk135 commented Jan 16, 2021

@StefanoFrazzetto I have released version 4.10.0 with strict mode and required modifier features for the Configuration provider. Here is a doc: https://python-dependency-injector.ets-labs.org/providers/configuration.html#strict-mode-and-required-options

@StefanoFrazzetto
Copy link
Author

StefanoFrazzetto commented Jan 16, 2021

Hi @rmk135 - that's great, thank you!

Just to be sure, could you tell me if the behaviour for the following is the expected one?

I updated the code to use strict=True in ServiceContainer's config

class ServiceContainer(containers.DeclarativeContainer):
    config = providers.Configuration(strict=True)
    myservice = providers.Singleton(MyService, config=config.myservice.required())


class Container(containers.DeclarativeContainer):
    config = providers.Configuration()
    services = providers.Container(ServiceContainer, config=config.services)
    mydevice = providers.Factory(MyDevice)

but I still get the attribute error in myservice.__init__()

Traceback (most recent call last):
  File "main.py", line 64, in <module>
    execute(broken)
  File "main.py", line 52, in execute
    mydevice.do_something()
  File "main.py", line 23, in do_something
    self.service().trigger()
  File "src/dependency_injector/providers.pyx", line 168, in dependency_injector.providers.Provider.__call__
  File "src/dependency_injector/providers.pyx", line 2263, in dependency_injector.providers.Singleton._provide
  File "src/dependency_injector/providers.pxd", line 552, in dependency_injector.providers.__factory_call
  File "src/dependency_injector/providers.pxd", line 538, in dependency_injector.providers.__callable_call
  File "src/dependency_injector/providers.pxd", line 528, in dependency_injector.providers.__call
  File "main.py", line 8, in __init__
    self.key = config.pop('key')
AttributeError: 'NoneType' object has no attribute 'pop'

Per documentation, I would expect the error to be raised earlier

You can use configuration provider in strict mode. In strict mode configuration provider raises an error on access to any undefined option.

Of course, it works fine if I do the following instead

class ServiceContainer(containers.DeclarativeContainer):
    config = providers.Configuration()
    myservice = providers.Singleton(MyService, config=config.myservice)


class Container(containers.DeclarativeContainer):
    config = providers.Configuration()
    services = providers.Container(ServiceContainer, config=config.services.required())
    mydevice = providers.Factory(MyDevice)
  [...]
  File "src/dependency_injector/providers.pyx", line 1486, in dependency_injector.providers.Configuration.get
dependency_injector.errors.Error: Undefined configuration option "config.services"

For convenience, I updated the gist.

@StefanoFrazzetto
Copy link
Author

StefanoFrazzetto commented Jan 16, 2021

Re: config.from_yaml('not_existent.yaml')

I would usually expect to have an exception raised when trying to do something with non-existent resources. If you believe it's better to keep that behaviour, then it might be useful to have a strict kwarg flag checking here, e.g.

try:
    with open(filepath) as opened_file:
        config = yaml.load(opened_file, yaml.Loader)
except OSError as e:
    if self.__strict and e.errno in (errno.ENOENT, errno.EISDIR):
        e.strerror = f"Unable to load yaml file ({e.strerror})"
        raise

I understand that, to be consistent, this would require making changes in the other methods as well, e.g. from_ini, so maybe I'll try to open a PR when possible.

PS: The above was inspired by Flask.

StefanoFrazzetto pushed a commit to StefanoFrazzetto/symbiotic that referenced this issue Jan 17, 2021
@rmk135
Copy link
Member

rmk135 commented Jan 20, 2021

Hey @StefanoFrazzetto ,

I updated the code to use strict=True in ServiceContainer's config
but I still get the attribute error in myservice.__init__()
For convenience, I updated the gist.

It was a bug. Fixed in 4.10.3.

Also you can give a specific name for the config provider in ServiceContainer to produce the most verbose error:

class ServiceContainer(containers.DeclarativeContainer):
    config = providers.Configuration('config.services', strict=True)

    myservice = providers.Singleton(MyService, config=config.myservice)


class Container(containers.DeclarativeContainer):
    config = providers.Configuration()

    services = providers.Container(ServiceContainer, config=config.services)

    mydevice = providers.Factory(MyDevice)

produces:

dependency-injector v4.10.3

### WORKING
> Device: triggering service
> Service: success

### BUG
> Device: triggering service
Traceback (most recent call last):
  File "/Users/rmoh/projects/ets-labs/python-dependency-injector/.workspace/issue358/bug2.py", line 64, in <module>
    execute(broken)
  File "/Users/rmoh/projects/ets-labs/python-dependency-injector/.workspace/issue358/bug2.py", line 52, in execute
    mydevice.do_something()
  File "/Users/rmoh/projects/ets-labs/python-dependency-injector/.workspace/issue358/bug2.py", line 23, in do_something
    self.service().trigger()
  File "src/dependency_injector/providers.pyx", line 168, in dependency_injector.providers.Provider.__call__
  File "src/dependency_injector/providers.pyx", line 2265, in dependency_injector.providers.Singleton._provide
  File "src/dependency_injector/providers.pxd", line 552, in dependency_injector.providers.__factory_call
  File "src/dependency_injector/providers.pxd", line 538, in dependency_injector.providers.__callable_call
  File "src/dependency_injector/providers.pxd", line 497, in dependency_injector.providers.__call
  File "src/dependency_injector/providers.pxd", line 389, in dependency_injector.providers.__provide_keyword_args
  File "src/dependency_injector/providers.pxd", line 312, in dependency_injector.providers.__get_value
  File "src/dependency_injector/providers.pyx", line 168, in dependency_injector.providers.Provider.__call__
  File "src/dependency_injector/providers.pyx", line 1233, in dependency_injector.providers.ConfigurationOption._provide
  File "src/dependency_injector/providers.pyx", line 1478, in dependency_injector.providers.Configuration.get
dependency_injector.errors.Error: Undefined configuration option "config.services.myservice"

PS: I will check if I could make config provider name definition config = providers.Configuration('config.services') to work automatically.

@rmk135
Copy link
Member

rmk135 commented Jan 20, 2021

Re: config.from_yaml('not_existent.yaml')
I would usually expect to have an exception raised when trying to do something with non-existent resources.

I agree. This is a design error. It didn't look like that when I designed it initially. Sort of hindsight bias :)

I can not change config.from_yaml('not_existent.yaml') to raise an error cause it will change current behaviour. There might be people who rely on current behaviour and their code will crash.

To fix the issue I could introduce a new API. Something like:

  • config.from_yaml('not_existent.yaml', required=True)
  • config.from_ini('not_existent.yaml', required=True)
  • config.from_env('UNDEFINED', required=True)
  • config.from_dict({}, required=True)

What do you think?

@Minitour
Copy link

Hi,

I have been experiencing problems with the Configuration Provider as well. The weird thing is that it runs perfectly fine locally on my machine, but when deployed to AWS Lambda, it seems to fail.

AttributeError: 'NoneType' object has no attribute 'get'
at func_wrapper (/var/task/xxxxxxxx/request_handler.py:52)
at ingest_kinesis (/var/task/xxx/index.py:53)
at dependency_injector.providers.Provider.__call__ (src/dependency_injector/providers.pyx:158)
at dependency_injector.providers.Singleton._provide (src/dependency_injector/providers.pyx:2124)
at dependency_injector.providers.__factory_call (src/dependency_injector/providers.pxd:424)
at dependency_injector.providers.__callable_call (src/dependency_injector/providers.pxd:410)
at dependency_injector.providers.__call (src/dependency_injector/providers.pxd:400)
at dependency_injector.providers.__provide_keyword_args (src/dependency_injector/providers.pxd:346)
at dependency_injector.providers.__get_value (src/dependency_injector/providers.pxd:278)
at dependency_injector.providers.Provider.__call__ (src/dependency_injector/providers.pyx:158)
at dependency_injector.providers.Singleton._provide (src/dependency_injector/providers.pyx:2124)
at dependency_injector.providers.__factory_call (src/dependency_injector/providers.pxd:424)
at dependency_injector.providers.__callable_call (src/dependency_injector/providers.pxd:410)
at dependency_injector.providers.__call (src/dependency_injector/providers.pxd:400)
at dependency_injector.providers.__provide_keyword_args (src/dependency_injector/providers.pxd:346)
at dependency_injector.providers.__get_value (src/dependency_injector/providers.pxd:278)
at dependency_injector.providers.Provider.__call__ (src/dependency_injector/providers.pyx:158)
at dependency_injector.providers.Singleton._provide (src/dependency_injector/providers.pyx:2124)
at dependency_injector.providers.__factory_call (src/dependency_injector/providers.pxd:424)
at dependency_injector.providers.__callable_call (src/dependency_injector/providers.pxd:410)
at dependency_injector.providers.__call (src/dependency_injector/providers.pxd:400)
at dependency_injector.providers.__provide_keyword_args (src/dependency_injector/providers.pxd:346)
at dependency_injector.providers.__get_value (src/dependency_injector/providers.pxd:278)
at dependency_injector.providers.Provider.__call__ (src/dependency_injector/providers.pyx:158)
at dependency_injector.providers.ConfigurationOption._provide (src/dependency_injector/providers.pyx:1133)

I am initializing the configuration provider using from_dict however I am doing this from within the container itself which doesn't align with the examples in the documentation but never the less was working (on my machine that is).

Locally I was running on Python 3.6, using Anaconda on Windows OS.

@rmk135
Copy link
Member

rmk135 commented Jan 20, 2021

Hi @Minitour , can you provide a code sample?

@Minitour
Copy link

Minitour commented Jan 21, 2021

@rmk135 Sure

class Container(containers.DeclarativeContainer):
    config = providers.Configuration()
    config.from_dict({
        'region': Const.REGION,    # <--- This is a string
        'ssm': Const.PARAMETERS # <--- This is a dict
    })
    ssm_client = providers.Singleton(boto3.client, 'ssm', region_name=config.region)
    s3_client = providers.Singleton(boto3.resource, 's3')
    parameter_kit = providers.Singleton(ParameterKit, ssm=ssm_client, parameters_config=config.ssm)
    database = providers.Singleton(DynamoDatabase)
    ingestion_config = providers.Singleton(IngestionConfig,
                                           s3_client=s3_client,
                                           parameter_kit=parameter_kit,
                                           db=database)

In lambda handler:

def handle(event, context):
     config = Container().ingestion_config()  # <------ This line crashes in AWS Lambda
     ...

@StefanoFrazzetto
Copy link
Author

StefanoFrazzetto commented Jan 21, 2021

@rmk135 As the proverb says, "hindsight is 20/20" 😄

Using a required flag seems a great option, though I think this should be passed with kwargs, as this would allow passing things like a custom yaml loader (I will open a new issue for a potential problem I spotted).

By the way, I still believe an exception should be raised when using a strict config.
Maybe that's something you could consider with 5.x.x?

PS: Sorry I can't actively help with this (e.g. by opening a PR), so thank you again for your work!

@rmk135
Copy link
Member

rmk135 commented Jan 21, 2021

Thanks @Minitour. I'll try to run a couple of experiments with it.

@rmk135
Copy link
Member

rmk135 commented Jan 21, 2021

@StefanoFrazzetto

I've created an issue for the required option: #369

By the way, I still believe an exception should be raised when using a strict config.
Maybe that's something you could consider with 5.x.x?

I think I can add this in version 4 since strict is a new feature. I'll use your code sample from #358 (comment). Here is an issue for this: #370

In version 5, strict=True should become a default.

@rmk135
Copy link
Member

rmk135 commented Jan 27, 2021

@Minitour ,

Here is a workaround for your problem:

def handle(event, context):
    container = Container()
    config = container.ingestion_config()  # <------ This line crashes in AWS Lambda
    ...

The root cause of that issue is kind of tricky. I'll check later if there is a good way to fix it.

@rmk135
Copy link
Member

rmk135 commented Jan 28, 2021

@Minitour ,

Fixed in 4.11.3.

def handle(event, context):
     config = Container().ingestion_config()  # <------ This line does not crash in AWS Lambda anymore
     ...

Thanks for reporting the problem.
I'm closing this issue. Feel free to re-open or comment if needed.

@rmk135 rmk135 closed this as completed Jan 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants