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

Allow getting session from the context. #129

Merged
merged 3 commits into from May 29, 2018

Conversation

@gtxm
Copy link

commented May 27, 2018

Hey,

Sometimes you are not able to provide the session to the Meta class and you want to mix marshmallow schema with marshmallow-sqlalchemy. The only place where you can put the session is the context (but you do it is up to you). With this PR you can override the session property to include session from context (example included in tests).

Cheers

Maciej Baranski added some commits May 27, 2018

Maciej Baranski
@sloria
Copy link
Member

left a comment

Thanks for the PR! Just one comment to address, and this should be good to merge.

@@ -137,11 +137,14 @@ class Meta:
"""
OPTIONS_CLASS = ModelSchemaOpts

@property
def session(self):
return self._session or self.opts.sqla_session

This comment has been minimized.

Copy link
@sloria

sloria May 28, 2018

Member

This is a breaking change since session was a public, write-able attribute. Adding a setter should make this backwards-compatible.

Maciej Baranski
@gtxm

This comment has been minimized.

Copy link
Author

commented May 28, 2018

Hey, thanks for the comment, I forgot about it, now it should be ok. 😄

@sloria

sloria approved these changes May 29, 2018

@sloria sloria merged commit c091b9f into marshmallow-code:dev May 29, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@gtxm gtxm referenced this pull request May 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.