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

zope.sqlalchemy is still using SessionExtension #31

Closed
zzzeek opened this issue Nov 24, 2018 · 3 comments
Closed

zope.sqlalchemy is still using SessionExtension #31

zzzeek opened this issue Nov 24, 2018 · 3 comments

Comments

@zzzeek
Copy link
Contributor

zzzeek commented Nov 24, 2018

Hi there -

Figured I'd take a look at this to see what the story is. So, part of the blame is on me since apparently I haven't had SessionExtension / MapperExtension / etc. emit deprecation warnings, I'm going to see if I can get that in for 1.3. However the Extension classes have been documented as deprecated since 2012.

It looks like, and I think I probably helped with this, that you have built up the register system that uses the (not really new anymore) event API. So, that has to be where this goes, because SessionExtension as well as the extension parameter on Session will go away in a future relase, (I don't want to say what release, but there is a rumor that it may not include the digit "1" inside of it).

So I think the methods of ZopeTransactionExtension need to be separated off of SessionExtension so you can still use them in register, then the actual ZopeTransactionExtension can be isolated from it. Then you will want to update your README to document the register() version of things as how people should be doing it (which has been for six years now :) ). Again I hope to get some deprecation warnings in 1.3 to help this all along.

Blame is on me for not emitting deprecation warnings for the extension parameter as well as use of SessionExtension etc., I'm going to add that to my list of todos.

@rouge8 rouge8 pinned this issue Mar 29, 2019
@rouge8 rouge8 unpinned this issue Mar 29, 2019
@icemac
Copy link
Member

icemac commented Sep 4, 2019

The deprecation warnings are landed in SQLAlchemy 1.3 so a volunteer is needed to update zope.sqlalchemy so it can run without the deprecated code.

@icemac
Copy link
Member

icemac commented Sep 11, 2019

Could the solution be as easy as dropping the parent class from ZopeTransactionExtension?

class ZopeTransactionExtension(SessionExtension):

We already use event.listen.
Maybe ZopeTransactionExtension should be renamed too, so users are pushed towards using the register function.

@ctheune ctheune self-assigned this Oct 12, 2019
@ctheune
Copy link
Contributor

ctheune commented Oct 12, 2019

Yeah, @icemac that was pretty much exactly the solution I ended up with ... :)

ctheune added a commit that referenced this issue Oct 16, 2019
…tion

Fix #31: stop using the deprecated "extension" way of registering events.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants