Conversion errors with `SQLAlchemy==2.0.2`

[kqf]

Conversion errors with `SQLAlchemy==2.0.2`

[kqf]

Oh, then it was a pure luck that it worked for my setup 😅

@lafrech many thanks for taking initiative for that. Je te souhaite bonne chance 🤞

[Inn0ve]

Ran into the same issue and wrote this SO question. I tried using configure_mappers() before loading my models as well:

from sqlalchemy.orm import configure_mappers
configure_mappers()
from models import *
from views import *

This however did not work and still gave me errors when trying to derive marshmallow-sqlalchemy schemas. However I was successfull in explicitly iterating the relationship objects which I believe may cause the same side effect:

mapper = inspect(cls)
_ = [_ for _ in mapper.relationships]

The configure_mappers() should be called after importing SQLAlchemy models, where the relationships are defined, but before the Schemas are imported.

In my particular setup, I have a models folder and a schema folders and things get initialized in that order:

models/
init.py -> this will import all the models from each of the models file, I have ~ 1 file per table. Once the models are imported, it will import schemas. (see init below)

schemas/
init.py -> This will first configure mappers at the top, then import all the schemas from their respective files.

Regardless if your file structure, calling configure_mappers() before the models are imported will not work.

[Inn0ve]

@lafrech

The following #495 fixes the issue with the lack of implicit calls to registry.configure() for both iterate_properties and get_property. It now passes all tests.

Feel free to take a look -- and also unless there are any other compatibility issues with SQLAlchemy 2.0, perhaps we can also adjust the requirements as well. I haven't tested anything beyond the unit tests already available.

[Inn0ve]

Rhynzler above is right.

I have faced also issues with relationships due to this. For anyone else facing this issue, for now my cleanest option has been to call configure_mappers() before importing my schemas.

from sqlalchemy.orm import configure_mappers
configure_mappers()

[schalkje]

Splitting model from schema's and calling configure_mappers(); worked for me

[Inn0ve]

@rhynzler's comment points to SQLAlchemy's changelog which seems to indicate that we'd rather be using Mapper.attrs. I have no time right now to investigate this but would @LloydCapson or anyone be willing to try this?

I've tried this before, it works fine as it does an implicit call to registry.configure(). However, it may be more a patch than anything, and may break again in the future. That being said, it definitely works.

It is equivalent to my suggestion above of calling configure_mappers(), but implicitly. For Marshmallow users, that is definitely a better patch. We do need to keep in mind that if SQLAlchemy phased that one out, we may eventually run into more issues, but we can kick the bucket down the road.

I can submit a PR for this if we want to go along with it. It's a very minor modification.

[lafrech]

Thanks @ddoyon92.

I was wrong in my comment: the method we used was not underscored. It's strange that it was removed but there must be a rationale for it. Anyway, following the advice in the changelog and using .attrs is the way to go.

I'll review and merge.

We may want to double-check the migration guide before finalizing a SQlAlchemy 2.x compatible release.

[lafrech]

Oh, right. Thanks for the feedback, then. Release should come soon.

[lafrech]

I'm afraid SQLA 2 is not supported at all (CI fails with 2.0). See #488.

[rhynzler]

The problem is because in the fields_for_model method of the ModelConverter class iterates over _orm.Mapper.iterate_properties which no longer implicitly calls _orm.registry.configure() in version 2.0.2. A temporary solution could be to modify the iteration so that it goes through _orm.Mapper.attrs which does implicitly call _orm.registry.configure()

Ref: https://docs.sqlalchemy.org/en/20/changelog/changelog_20.html#change-45fe5664c88893d899c991ca3675142c

[YuvalItzchakov]

Ran into the same issue and wrote this SO question. I tried using configure_mappers() before loading my models as well:

from sqlalchemy.orm import configure_mappers
configure_mappers()
from models import *
from views import *

This however did not work and still gave me errors when trying to derive marshmallow-sqlalchemy schemas.
However I was successfull in explicitly iterating the relationship objects which I believe may cause the same side effect:

mapper = inspect(cls)
_ = [_ for _ in mapper.relationships]

[LloydCapson]

I've got a pull request in that fixes this issue. https://github.com/marshmallow-code/marshmallow-sqlalchemy/pull/491

[lafrech]

Thanks @LloydCapson. Please rebase your PR. Also please reword the comment (it sounds like we're blaming SQLA maintainers). Anyone here, #491 PR review welcome.

Thanks!

[LloydCapson]

Ok I believe I've got it rebased correctly and cleaned up. Let me know if there's anything wrong with it and I'll fix it. I'm making an effort to learn to contribute.

[YuvalItzchakov]

Ran into the same issue and wrote this SO question. I tried using configure_mappers() before loading my models as well:

from sqlalchemy.orm import configure_mappers
configure_mappers()
from models import *
from views import *

This however did not work and still gave me errors when trying to derive marshmallow-sqlalchemy schemas. However I was successfull in explicitly iterating the relationship objects which I believe may cause the same side effect:

mapper = inspect(cls)
_ = [_ for _ in mapper.relationships]

The configure_mappers() should be called after importing SQLAlchemy models, where the relationships are defined, but before the Schemas are imported.

In my particular setup, I have a models folder and a schema folders and things get initialized in that order:

models/
init.py -> this will import all the models from each of the models file, I have ~ 1 file per table. Once the models are imported, it will import schemas. (see init below)

schemas/
init.py -> This will first configure mappers at the top, then import all the schemas from their respective files.

Regardless if your file structure, calling configure_mappers() before the models are imported will not work.

The issue I have with that approach is that my models have a decorator to them that automagically derives the marshmallow schema. I can derive all models at the end by iterating them all, would just be a bit less convenient.

[Inn0ve]

Ran into the same issue and wrote this SO question. I tried using configure_mappers() before loading my models as well:

from sqlalchemy.orm import configure_mappers
configure_mappers()
from models import *
from views import *

This however did not work and still gave me errors when trying to derive marshmallow-sqlalchemy schemas. However I was successfull in explicitly iterating the relationship objects which I believe may cause the same side effect:

mapper = inspect(cls)
_ = [_ for _ in mapper.relationships]

The configure_mappers() should be called after importing SQLAlchemy models, where the relationships are defined, but before the Schemas are imported.
In my particular setup, I have a models folder and a schema folders and things get initialized in that order:
models/
init.py -> this will import all the models from each of the models file, I have ~ 1 file per table. Once the models are imported, it will import schemas. (see init below)
schemas/
init.py -> This will first configure mappers at the top, then import all the schemas from their respective files.
Regardless if your file structure, calling configure_mappers() before the models are imported will not work.

The issue I have with that approach is that my models have a decorator to them that automagically derives the marshmallow schema. I can derive all models at the end by iterating them all, would just be a bit less convenient.

I see. On my side I avoid using the decorators because I almost always need to do something custom at the schema level. I personally use the SQLAlchemyAutoSchema to "automatically" generate the schemas, although it still requires a few lines of code. However, I think those can be more easily extended with additional functionalities.

Something like:
https://marshmallow-sqlalchemy.readthedocs.io/en/latest/

If ever you are still not happy with your current solution and can't wait for the next version of marshmallow, you could try something similar. The footprint is larger than a simple decorator, but I think there are also other benefits... Either way, if your solution works, that's fine too. Eventually Marshmallow will just be patched.

[lafrech]

@rhynzler's comment points to SQLAlchemy's changelog which seems to indicate that we'd rather be using Mapper.attrs. I have no time right now to investigate this but would @LloydCapson or anyone be willing to try this?

[Inn0ve]

@lafrech @LloydCapson

Swapping the model.mapper.iterate_properties for model.mapper.attrs works fine, and is equivalent to how it worked before. We do not need to explicitly make a call to _check_configure() as in the PR currently presented.

That being said, while most tests are now passing -- we are still getting some tests failures that should be investigated, which are the same whether we use _check_configure() or the mapper.attrs:

FAILED tests/test_conversion.py::TestFieldFor::test_field_for - AttributeError: columns ERROR tests/test_sqlalchemy_schema.py::test_load[sqla_schema_with_relationships] - AttributeError: columns ERROR tests/test_sqlalchemy_schema.py::TestLoadInstancePerSchemaInstance::test_toggle_load_instance_per_schema[schema_no_load_instance] - AttributeError: columns ERROR tests/test_sqlalchemy_schema.py::TestLoadInstancePerSchemaInstance::test_toggle_load_instance_per_schema[schema_with_load_instance] - AttributeError: columns ERROR tests/test_sqlalchemy_schema.py::test_load_validation_errors[sqla_schema_with_relationships] - AttributeError: columns

[lafrech]

I guess the original sin was to use an underscore method. This broke in a SQLAlchemy patch release. (Luckily, this was on SQLAlchemy 2.0 which is not supported yet anyway.)

Let's not make the same mistake twice. If it is equivalent to use .attrs as advised in SQLAlchemy's changelog, go for it.

Thank you guys for investigating this.

[lafrech]

Tests pass on SQLAlchemy 2.0. I also tested in my code base.

Would you guys like to test in your code bases before we ship?

[Inn0ve]

I've tested against our codebase, everything works as expected and our test suite is passing. I think this particular change is fairly safe.

[lafrech]

Yes I also think the change in #495 is safe.

My point is to test your code base against against SQLAlchemy 2.0 (with latest version in dev here) before we ship the new version with official SQLALchemy 2.0 support.

[Inn0ve]

Ah I see. We've already migrated to SQLAlchemy 2.0.4 --> I had already implemented the configure_mapper() call in our codebase before importing the schemas, and that was our only problem encountered during migration. I simply removed that patch on our side, and tested the latest dev version instead. That worked fine.

For our use case, we did not encounter any other issues with SQLAlchemy 2.0+ except for that one which we addressed.