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

Use ModelSchema.session setter rather than OPTIONS_CLASS #111

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

antgel
Copy link

@antgel antgel commented Jan 19, 2019

When I use webargs with flask-marshmallow, there is an issue whereby webargs calls schema.load(parsed), and fails because the DummySession is returned instead of the sqlalchemy scoped session.

I am relying on init_app() to bind the sqlalchemy session. I can see from other issues that people have suggested passing sqla_session in Meta, but I'd rather not copy and paste boilerplate if I can avoid it. Looking at the relevant line in marshmallow-sqlalchemy, it looks like ModelSchema.load expects self._session to be set, which ModelSchema.OPTIONS_CLASS.session doesn't do, but the setter does.

The tests still pass, but as a new user of this ecosystem, I'd be happy to know if I've missed something. In particular, I know a lot of people use this stuff, so I'm not sure how it works for others.

Copy link
Member

@sloria sloria left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does look like the current code is incorrect, I'm not sure this is the right approach. This isn't actually using the setter; it's overriding the session attribute on the ModelSchema class, which means that the sqla_session class Meta option won't be respected.

This will require more investigation. I don't have time to look into it more closely right now. In the meantime, it'd be helpful to have some test cases, or even just a code snippet to reproduce the issue.

@ZinkLu
Copy link

ZinkLu commented May 5, 2019

I have the DummySession problem too, but I don't think the solution is to set self.ModelSchema.session = db.session cause it is not possible to get sqla_session in Meta again.
I found that I use init_app manually after I create Marshmallow instance (I create flask app with factory mode), so the subclasses of ma.ModelSchema were created before init_app is called, thus the self.ModelSchema.OPTIONS_CLASS.session is always DummySession instance when the subclasses were created . My solution is to set ma.ModelSchema.OPTIONS_CLASS.session = db.session right after create the SQLAlchemy instance and the Marshmallow instance. It works for me, but the init_app also should be change.

bouttier added a commit to PnX-SI/GeoNature that referenced this pull request Oct 17, 2022
ma.init_app(app) replace DummySession() by db.session but its too late,
schemas have already been created, so replace it as soon as ma have been initialized.

marshmallow-code/flask-marshmallow#44
marshmallow-code/flask-marshmallow#74
marshmallow-code/flask-marshmallow#111
bouttier added a commit to PnX-SI/GeoNature that referenced this pull request Oct 17, 2022
ma.init_app(app) replace DummySession() by db.session but its too late,
schemas have already been created, so replace it as soon as ma have been initialized.

marshmallow-code/flask-marshmallow#44
marshmallow-code/flask-marshmallow#74
marshmallow-code/flask-marshmallow#111
bouttier added a commit to PnX-SI/GeoNature that referenced this pull request Oct 17, 2022
ma.init_app(app) replace DummySession() by db.session but its too late,
schemas have already been created, so replace it as soon as ma have been initialized.

marshmallow-code/flask-marshmallow#44
marshmallow-code/flask-marshmallow#74
marshmallow-code/flask-marshmallow#111
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants