FactoryBoy / factory_boy

A test fixtures replacement for Python
https://factoryboy.readthedocs.io/
MIT License
3.52k stars 398 forks source link

Unable to query db inside factory ('Iterator' object has no attribute '_sa_instance_state') #445

Open fgblomqvist opened 6 years ago

fgblomqvist commented 6 years ago

I got this factory:

class DepartmentFactory(factory.alchemy.SQLAlchemyModelFactory):
    class Meta:
        model = models.Department
        sqlalchemy_session = Session

    abbreviation = 'MATH'

    @factory.lazy_attribute
    def school(self):
        return factory.Iterator(Session.query(models.School).all())

And I get this error: AttributeError: 'Iterator' object has no attribute '_sa_instance_state'

I've checked and made sure that the query returns a list of School objects.

fgblomqvist commented 6 years ago

Forgot to mention that the reason why I'm posting this on the bug tracker is because there is an example in the docs that does exactly this, so I assume it should work.

rbarrois commented 6 years ago

@fgblomqvist this looks like a bug indeed ;)

Would you have a more complete stack trace? We'd need to know what is accessing your object's _sa_instance_state.

And I'll ping @jeffwidman on this one — he is much more knowledgeable on SQLAlchemy than me :)

fgblomqvist commented 6 years ago

Here's a stack trace:

Traceback (most recent call last):
  File "/usr/lib/python3.6/unittest/case.py", line 59, in testPartExecutor
    yield
  File "/usr/lib/python3.6/unittest/case.py", line 601, in run
    self.setUp()
  File ".../tests/resources/test_quarters.py", line 20, in setUp
    c1 = CourseFactory(id=1000)
  File ".../lib/python3.6/site-packages/factory/base.py", line 46, in __call__
    return cls.create(**kwargs)
  File ".../lib/python3.6/site-packages/factory/base.py", line 568, in create
    return cls._generate(enums.CREATE_STRATEGY, kwargs)
  File ".../lib/python3.6/site-packages/factory/base.py", line 505, in _generate
    return step.build()
  File ".../lib/python3.6/site-packages/factory/builder.py", line 272, in build
    step.resolve(pre)
  File ".../lib/python3.6/site-packages/factory/builder.py", line 221, in resolve
    self.attributes[field_name] = getattr(self.stub, field_name)
  File ".../lib/python3.6/site-packages/factory/builder.py", line 363, in __getattr__
    extra=declaration.context,
  File ".../lib/python3.6/site-packages/factory/declarations.py", line 306, in evaluate
    return self.generate(step, defaults)
  File ".../lib/python3.6/site-packages/factory/declarations.py", line 395, in generate
    return step.recurse(subfactory, params, force_sequence=force_sequence)
  File ".../lib/python3.6/site-packages/factory/builder.py", line 233, in recurse
    return builder.build(parent_step=self, force_sequence=force_sequence)
  File ".../lib/python3.6/site-packages/factory/builder.py", line 279, in build
    kwargs=kwargs,
  File ".../lib/python3.6/site-packages/factory/base.py", line 314, in instantiate
    return self.factory._create(model, *args, **kwargs)
  File ".../lib/python3.6/site-packages/factory/alchemy.py", line 75, in _create
    obj = model_class(*args, **kwargs)
  File "<string>", line 4, in __init__
  File ".../lib/python3.6/site-packages/sqlalchemy/orm/state.py", line 417, in _initialize_instance
    manager.dispatch.init_failure(self, args, kwargs)
  File ".../lib/python3.6/site-packages/sqlalchemy/util/langhelpers.py", line 66, in __exit__
    compat.reraise(exc_type, exc_value, exc_tb)
  File ".../lib/python3.6/site-packages/sqlalchemy/util/compat.py", line 187, in reraise
    raise value
  File ".../lib/python3.6/site-packages/sqlalchemy/orm/state.py", line 414, in _initialize_instance
    return manager.original_init(*mixed[1:], **kwargs)
  File ".../lib/python3.6/site-packages/sqlalchemy/ext/declarative/base.py", line 700, in _declarative_constructor
    setattr(self, k, kwargs[k])
  File ".../lib/python3.6/site-packages/sqlalchemy/orm/attributes.py", line 229, in __set__
    instance_dict(instance), value, None)
  File ".../lib/python3.6/site-packages/sqlalchemy/orm/attributes.py", line 831, in set
    value = self.fire_replace_event(state, dict_, value, old, initiator)
  File ".../lib/python3.6/site-packages/sqlalchemy/orm/attributes.py", line 853, in fire_replace_event
    self._replace_token or self._init_append_or_replace_token())
  File ".../lib/python3.6/site-packages/sqlalchemy/orm/attributes.py", line 1199, in emit_backref_from_scalar_set_event
    child_state, child_dict = instance_state(child),\
AttributeError: 'Iterator' object has no attribute '_sa_instance_state'

My guess is that it has something to do with factory.Iterator since I am able to query just one object (e.g. .first()) and just return that without an error, but that's just a guess :P

rbarrois commented 6 years ago

OK, I think i got it.

With:

@factory.lazy_attribute
def school(self):
     return foo

When building or creating an object, we'll end up with .create(school=foo) (foo being the value returned by this call). Since you return a factory.Iterator, this is equivalent to calling Department(school=factory.Iterator(...)) — SQLAlchemy doesn't know what to do with that factory.Iterator.

You should simply replace that block with:

school = factory.Iterator(Session.query(models.School).all())
fgblomqvist commented 6 years ago

I wish it was that simple, unfortunately, the query then gets executed before the session exists. What I think is odd here is that a lazy attribute cannot return the same thing as a non-lazy attribute. Isn't that the real bug? What should happen (according to me) is that the lazy decorator should do whatever factory_boy normally does to whatever is returned. E.g. if an iterator is returned, call iterator.evaluate() etc. (or whatever is usually done with an iterator).

Looking at the code, what you need to do is wrap the function decorated by lazy, in the function that normally evaluates an attribute. Then everything should be fine.

rbarrois commented 6 years ago

@fgblomqvist well, that's a core design architecture of factory_boy since day one ;) Changing it would have significant impacts.

However, for your use case, I think that the solution is factory.iterator:

@factory.iterator
def school():
    return Session.query(models.School).all()
fgblomqvist commented 6 years ago

Hmm I see the issue now with the core design. Oh well, since the lazy decorator is needed (as in, what you proposed does not work), I just went with this:

@factory.lazy_attribute
def school(self):
    return random.choice(models.School.query.all())

Not as good, but works for my purpose at least.

rbarrois commented 6 years ago

Well, actually, @factory.iterator not being lazy is definitely a bug!

I'm pretty sure it works with Django :(

Can you share the stacktrace you get when calling with @factory.iterator?

fgblomqvist commented 6 years ago

Ah okay. Here is the stacktrace:

Traceback (most recent call last):
  File "/usr/lib/python3.6/unittest/loader.py", line 462, in _find_test_path
    package = self._get_module_from_name(name)
  File "/usr/lib/python3.6/unittest/loader.py", line 369, in _get_module_from_name
    __import__(name)
  File ".../tests/__init__.py", line 10, in <module>
    from tests.fixtures.factories import StudentFactory, MajorFactory
  File ".../tests/fixtures/factories/__init__.py", line 1, in <module>
    from .course import CourseFactory
  File ".../tests/fixtures/factories/course.py", line 4, in <module>
    from .department import DepartmentFactory
  File ".../tests/fixtures/factories/department.py", line 7, in <module>
    class DepartmentFactory(factory.alchemy.SQLAlchemyModelFactory):
  File ".../tests/fixtures/factories/department.py", line 15, in DepartmentFactory
    @factory.iterator
  File ".../lib/python3.6/site-packages/factory/helpers.py", line 102, in iterator
    return declarations.Iterator(func())
  File ".../tests/fixtures/factories/department.py", line 17, in school
    return models.School.query.all()
  File ".../lib/python3.6/site-packages/flask_sqlalchemy/__init__.py", line 514, in __get__
    return type.query_class(mapper, session=self.sa.session())
  File ".../lib/python3.6/site-packages/sqlalchemy/orm/scoping.py", line 74, in __call__
    return self.registry()
  File ".../lib/python3.6/site-packages/sqlalchemy/util/_collections.py", line 1001, in __call__
    return self.registry.setdefault(key, self.createfunc())
  File ".../lib/python3.6/site-packages/sqlalchemy/orm/session.py", line 2939, in __call__
    return self.class_(**local_kw)
  File ".../lib/python3.6/site-packages/flask_sqlalchemy/__init__.py", line 141, in __init__
    self.app = app = db.get_app()
  File ".../lib/python3.6/site-packages/flask_sqlalchemy/__init__.py", line 912, in get_app
    'No application found. Either work inside a view function or push'
RuntimeError: No application found. Either work inside a view function or push an application context. See http://flask-sqlalchemy.pocoo.org/contexts/.

The error at the end means that the session is not initialized. Looks like the function gets executed as soon as it's defined (see return declarations.Iterator(func())).

On a side note: what struck me as odd is that if you decorate the function with iterator, there is no self.

rbarrois commented 6 years ago

Oh, thanks for the stack!

Indeed, the factory.Iterator / factory.iterator functions work best with a generator function. It should work with:

@factory.iterator
def school():
    yield from Session.query(models.School).all()

Let's dive under the hood here: a generator function (anything containing yield) returns a special object, which will only execute the function's code (up to the next yield statement) once next(x) is called on it.

Django's querysets are built upon generators, and work natively; however, it seems that SQLAlchemy's querysets will evaluate directly; using a yield from x is equivalent to for i in x: yield i, and "wraps" the call in a generator.

This could be fixed either through better docs, or in the code; I'll have to see which way feels most natural.

Regarding the lack of self:

There is some "dark magic" occurring in factory_boy: basically, the engine will collect all attributes from your class definition, and store them in some internal structures for later use.

I use the self parameter to calls because it feels more natural, but that's actually a trick :wink: When creating an object, a method of those definitions is called with a stub for the attributes of the object to be built. Once that stub is filled, its attributes are used as kwargs for the actual model.

Since an iterator cannot be tuned for the object being built (whereas a lazy_attribute is designed for that!), there is no self parameter.

fgblomqvist commented 6 years ago

Turning it into a generator function fixed it, thanks for the extensive explanation. Will use that for now. Treating it differently (and wrapping it accordingly) depending on if it's an SQLAlchemy one or not feels the most natural to me (aka. the user does not have to care), but that is surely your decision to make.

Thanks for the in-depth explanation about the lack of self, the more you know! :smile: It was a bit of an eye-sore both for me and my IDE so solved it this way:

def schools():
    yield from models.School.query.all()

class DepartmentFactory(factory.alchemy.SQLAlchemyModelFactory):
    class Meta:
        model = models.Department
        sqlalchemy_session = models.db.session

    abbreviation = factory.Iterator(['COEN', 'MATH', 'POLI', 'MECH'])
    name = 'Sample Department'
    school = factory.iterator(schools)

:stuck_out_tongue_winking_eye: