dgilland / sqlservice

The missing SQLAlchemy ORM interface.
https://sqlservice.readthedocs.io
MIT License
179 stars 9 forks source link

.save() behavior #4

Closed ohlookemus closed 7 years ago

ohlookemus commented 7 years ago

So maybe I'm not using this function right but in my application

    with database.transaction():  # handles rollback on transaction fail.
        experiment = TestObject(
              objectName='hello'
        )
        database.save(experiment)
        database.flush()

Throws the following error: Unexpected error. Error was: Type of value given to save() method is not a valid SQLALchemy declarative class. Item with index 0 and with value \"<TestObject(objectName='hello')>\" is an instance of \"<class 'orm.TestObject'>\"."

How my orm.TestObject is defined...

Model = declarative_base(MyBase, metadata=meta)

class TestObject(Model):
    objectName = Column(String())

However in unit tests when I emulate this behavior, it goes through fine.

    def testORMInsert(self):
          test = TestObj(**objData)
          self.db.save(test)
          self.assertFalse(test.id)
          self.db.commit()
          self.assertTrue(test.id)

Thoughts? Am I using it wrong or?

dgilland commented 7 years ago

How is database defined? What's the output of isinstance(TestObject, database.model_class)? If it's False, then whatever declarative base you passed to database isn't the one used with TestObject.

Another thing you can check is database.models.values() to see which ORM models are associated with the declarative base class. If TestObject is in database.models.values(), then would it be possible for you to provide a fully working example (i.e. runnable code) that demonstrates the issue?

ohlookemus commented 7 years ago

Actually figured this out shortly after I posted. Another developer had a different model that he also named TestObject in a different module that's loaded with the one I have(ie, my.test.TestObj and his.othermodulename.TestObj). Found out that models.values() didn't load either. Not sure if that's classified as intended or if an error or warning should be thrown when that happens.

Our unit tests only test the isolated TestObj which is why we never saw this until we had the application running. :)

dgilland commented 7 years ago

So you both defined a TestObj model class that both used the same declarative base model?

Something like the following:

# base.py
Model = declarative_base(MyBase)

# my.test.py
from ..base import Model
class TestObj(Model): pass

# his.other.py
from ..base import Model
class TestObj(Model): pass
ohlookemus commented 7 years ago

Correct.

dgilland commented 7 years ago

Looks like the way SQLAlchemy stores ORM objects with the same class name in its registry breaks some of the assumptions made in sqlservice about how to reference them. Essentially, SQLAlchemy stores these in an iterable object, sqlalchemy.ext.declarative.clsregistry._MultipleClassMarker, which would need to be "unpacked" with something like list(<MultipleClassMarker>).

So to make this type of usage (i.e. support duplicate class names defined in separate modules), I'd just need to unpack these MultipleClassMarker instances and add them to sqlservice's internal model map. But having duplicate class names would break how db.<Model> works as it would be an ambiguous reference to the underlying model class. I'll have to think more on this, but suffice to say I will be to at least make it so that db.save(my.TestObj) and db.save(his.TestObj) would work.

dgilland commented 7 years ago

Support for duplicate model class names defined in separate submodules is implemented in 2dbed0304bb0e3c8770359252e61f66f61e79589. Will be included in the next 0.10.0 release.

dgilland commented 7 years ago

Version 0.10.0 has been released with this fix: https://pypi.python.org/pypi/sqlservice/0.10.0