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

ModelSchema.load(instance=None) call wont overridden .instance #78

Closed
georgexsh opened this issue Aug 1, 2016 · 6 comments

Comments

@georgexsh
Copy link

commented Aug 1, 2016

as in ModelSchema.load

self.instance = instance or self.instance

because I instantiation schema class in module level, reuse them in multiple places and across requests,
the .instance value firstly set will persists in following load() calls, regardless load(instance=None), this will trigger nasty bug as its value depends on request order.

is there a good reason for this behavior and not just set self.instance from parameter?
or should I instantiation a new schema object before any load or dump call?

@blurrcat

This comment has been minimized.

Copy link

commented Aug 23, 2016

I'm experiencing the same problem and it costs me the whole afternoon. I feel there's no point keeping the instance on the schema.

@nielstuts

This comment has been minimized.

Copy link

commented Jan 5, 2017

Is it just me or is marshallow-sqlalchemy currently not usable when reusing the schema due to this bug? If you use the "load" method and pass through an instance (for example in a REST call that modifies a resource) the instance will then stick and it will use that instance for subsequent REST calls. Creating a new resource by using the load method will just reuse the instance and thus modify that instance instead of creating a new SQLAlchemy model. Here's some minimal code that's affected by this issue:

https://github.com/singingwolfboy/build-a-flask-api/tree/master/step13

Create a puppy, then edit it and finally create another new puppy. Instead of creating the second puppy, it will just edit the first one.

@blurrcat

This comment has been minimized.

Copy link

commented Jan 5, 2017

@nielstuts It's usable, just not like what you'd expect.
The instance persists in memory if you use it on a class/module level. Currently the only thing way to get around is not to re-use the schema instance for loading data.
Create new schema instances in your create/edit view.

@sloria sloria added the bug label Jan 5, 2017

@sloria sloria closed this in 1531189 Jan 5, 2017

@sloria

This comment has been minimized.

Copy link
Member

commented Jan 5, 2017

I've just released 0.12.1, which should fix this bug. Thanks for reporting! Please reopen this if the issue persists.

@ThiefMaster

This comment has been minimized.

Copy link

commented Apr 28, 2017

I'm not using threads but just from looking at the code: Even with resetting it, can't you end up with awful race conditions with multithreaded code there since a schema instance will be shared between threads?

@georgexsh

This comment has been minimized.

Copy link
Author

commented Jun 27, 2017

+1 for @ThiefMaster thread safe concerns, this should be documented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.