fedora-hubs

#481 Rationalize the instantiation of the DB session
Merged by abompard. Opened by abompard.
abompard/fedora-hubs fix/reorg-session  into  develop

Download 481.patch

This brings the usage closer to what SQLAlchemy docs recommend.

It also avoids some import loops, and passing the session around all the
time when outside of Flask's request context.

See:
- http://docs.sqlalchemy.org/en/latest/orm/contextual.html#unitofwork-contextual
- http://docs.sqlalchemy.org/en/latest/orm/session_basics.html#adding-additional-configuration-to-an-existing-sessionmaker

Looks like this needs to be rebased.

rebased onto 49de53360cfbbe0503cb83afe9e23b6d27059801

@bowlofeggs: done

I think you do want to autocommit upon successful request. I'm actually not sure what will happen to the session if you do not commit. It seems likely the changes would be lost, or worse, would persist in the next request? I have no idea, but I think you do want to commit.

FWIW, Bodhi commits upon each successful request:

https://github.com/fedora-infra/bodhi/blob/3.0.0/bodhi/server/init.py#L44-L70

You might want to also do Session.remove() here.

It would be good to give this a docblock.

It would be good to give this a docblock.

I noticed that you are passing a db session around in a few methods. Instead of doing that, you could take advantage of this query object. I've been trying to convert Bodhi to work that way for a while since it's a bit cleaner.

It'd be good to document the create parameter as well.

LGTM!

Thanks! I'll fix all that and merge it.

rebased onto 154fad92ca007e168ffabf1a7b790a1b9f1bb6e4

Pull-Request has been merged by abompard
Metadata
None
No Tags
Static archive. pagure.io was sunset in 2026; this is a read-only snapshot.
Red Hat Logo

Fedora is sponsored by Red Hat. Learn more about the relationship between Red Hat and Fedora »

© Red Hat, Inc. and others.