cosmicpython / code

Example application code for the python architecture book
Other
2.14k stars 946 forks source link

tests failing due to possible conflict with SQLAlchemy #17

Closed jmaralc closed 2 years ago

jmaralc commented 4 years ago

Hi, first of all thanks for the book . I was missing this kind of book for a lot of time. The thing is that runing pytest I've got some errors on the test_orm.py file:

==================================== test session starts ====================================
platform linux -- Python 3.7.3, pytest-5.4.1, py-1.8.1, pluggy-0.13.1
rootdir: /home/javier/Projects/p_patterns/code-chapter_02_repository
collected 20 items                                                                          

test_allocate.py ....                                                                 [ 20%]
test_batches.py ........                                                              [ 60%]
test_orm.py FF..FF                                                                    [ 90%]
test_repository.py .F                                                                 [100%]

...

___________________________ test_orderline_mapper_can_load_lines ____________________________

session = <sqlalchemy.orm.session.Session object at 0x7f484b9d9390>

    def test_orderline_mapper_can_load_lines(session):
        session.execute(
            'INSERT INTO order_lines (orderid, sku, qty) VALUES '
            '("order1", "RED-CHAIR", 12),'
            '("order1", "RED-TABLE", 13),'
            '("order2", "BLUE-LIPSTICK", 14)'
        )
        expected = [
>           model.OrderLine("order1", "RED-CHAIR", 12),
            model.OrderLine("order1", "RED-TABLE", 13),
            model.OrderLine("order2", "BLUE-LIPSTICK", 14),
        ]

test_orm.py:12: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
<string>:2: in __init__
    ???
../../../.local/share/virtualenvs/p_patterns-tDNksF9p/lib/python3.7/site-packages/sqlalchemy/orm/instrumentation.py:377: in _new_state_if_none
    self._state_setter(instance, state)
<string>:1: in set
    ???
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <[AttributeError("'OrderLine' object has no attribute '_sa_instance_state'") raised in repr()] OrderLine object at 0x7f484ba4c898>
name = '_sa_instance_state'
value = <sqlalchemy.orm.state.InstanceState object at 0x7f484b9d9dd8>

>   ???
E   dataclasses.FrozenInstanceError: cannot assign to field '_sa_instance_state'

I could not find much information about the error but it seems like passing an integer to an id that expects an object... like https://stackoverflow.com/questions/16151729/attributeerror-int-object-has-no-attribute-sa-instance-state?rq=1

But to tell the truth no idea whether that is the case. Do you suffer from the same issue?

Thanks in advance

hjwp commented 4 years ago

i think i know what it is. i should probably add a footnote in the book. but you need to change the @dataclass(frozen=True) to @dataclass(unsafe_hash=True). because sqlalchemy reasons.

jmaralc commented 4 years ago

Not sure about it. I'm taking the code from the repo and it is already defined as:

@dataclass(unsafe_hash=True)`
class OrderLine:
    orderid: str
    sku: str
    qty: int
hjwp commented 4 years ago

it's weird that you'd get a dataclasses.FrozenInstanceError then?

do you have a repo you can share?

jmaralc commented 4 years ago

Sorry Harry, my bad. It was failing in a copy of the model file with the frozen=True that I generated before by following the chapter. Sorry again for the extra work and thanks, diving in the hashes topics of dataclasses gave me a nice insight of what is happening.

tomdottom commented 3 years ago

After fighting SqlAlchemy and solving this before finding this issue I'd just like to say that you can use frozen dataclasses. However, you will have to provide a custom instrumentation manager.

Example:

For a frozen OrderLine dataclass

@dataclass(unsafe_hash=True)
class OrderLine:
    orderid: str
    sku: str
    qty: int

We can use a custom FrozenDataclassInstrumentationManager .

from sqlalchemy import Column, Integer, MetaData String, Table
from sqlalchemy.ext.instrumentation import InstrumentationManager
from sqlalchemy.orm import mapper

metadata = MetaData()

DEL_ATTR = object()

class FrozenDataclassInstrumentationManager(InstrumentationManager):
    def install_member(self, class_, key, implementation):
        self.originals.setdefault(key, class_.__dict__.get(key, DEL_ATTR))
        setattr(class_, key, implementation)

    def uninstall_member(self, class_, key):
        original = self.originals.pop(key, None)
        if original is not DEL_ATTR:
            setattr(class_, key, original)
        else:
            delattr(class_, key)

    def dispose(self, class_):
        del self.originals
        delattr(class_, "_sa_class_manager")

    def manager_getter(self, class_):
        def get(cls):
            return cls.__dict__["_sa_class_manager"]
        return get

    def manage(self, class_, manager):
        self.originals = {}
        setattr(class_, "_sa_class_manager", manager)

    def get_instance_dict(self, class_, instance):
        return instance.__dict__

    def install_state(self, class_, instance, state):
        instance.__dict__["state"] = state

    def remove_state(self, class_, instance, state):
        del instance.__dict__["state"]

    def state_getter(self, class_):
        def find(instance):
            return instance.__dict__["state"]
        return find

OrderLine.__sa_instrumentation_manager__ = FrozenDataclassInstrumentationManager

orderline_table = Table(
    "order_lines",
    metadata,
    Column("id", Integer, primary_key=True, autoincrement=True),
    Column("sku", String(128), nullable=False),
    Column("quantity", Integer, nullable=False),
)

mapper(
    OrderLine,
    orderline_table,
)

https://stackoverflow.com/a/66941711/2398354

nikoladsp commented 3 years ago

Hi, I hit the very same issue.

More to that, even with InstrumentationManager, dataclass is changed by SA: after mapping is performed, SA has added some of its internal attributes (_sa_instance_state and _sa_class_manager) to dataclass, so we can not speak about decoupling and this beats the purpose of Registry in the first place since dataclass only looks like its decoupled.

I am not sure if there is a clean way to do this with SA, and posted question here. Additionaly, turns out that InstrumentationManager is very old, untested and almost never used.

If anyone have some ideas, please share.

Thank you kindly

hjwp commented 2 years ago

as always it's a tradeoff. Yes, in order to work, SQLAlchemy has to get its claws pretty deep into your objects, so they are not "truly" decoupled. On the other hand, an ORM does a lot of the heavy lifting for getting objects in and out of database columns.

The alternative is not to use an ORM at all, which I've enjoyed doing on the last few projects. You have to write a lot of your own SQL tho! Check out the exercise for the reader here, so you can get a feel for the tradeoff http://www.cosmicpython.com/book/chapter_02_repository.html#api_endpoint_with_repo